Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Message-ID: <6c3ed3dd-25e5-18b7-fd17-d126975b6b50@oracle.com>
Date:   Mon, 30 Oct 2023 10:02:24 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.15.1
Subject: Re: [PATCH] scsi: sr: Fix a potential uninit-value in sr_get_events()
To:     Shigeru Yoshida <syoshida@redhat.com>, bvanassche@acm.org
Cc:     jejb@linux.ibm.com, martin.petersen@oracle.com,
        linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230531164346.118438-1-syoshida@redhat.com>
 <c459dd2c-a281-7fc4-90cf-36a71e9548bc@acm.org>
 <20231030.171317.1096475355592607869.syoshida@redhat.com>
Content-Language: en-US
From:   michael.christie@oracle.com
In-Reply-To: <20231030.171317.1096475355592607869.syoshida@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Y2pYQ0p1VFV5RXFxMzJXRlJJRWdsWDlvMjRKeVJmVkR5bDEvblk5VkpXQkVE?=
 =?utf-8?B?MWxNN2pJbVFxaStIeWx4V05zQU9nQ2xrVk1YU24wdDVTejZ6ZlNUYUtSWDZq?=
 =?utf-8?B?ZDN1NzZEbGlRbkY0Yjh0Nkw2bXdkTDdjTVVDOHp4Vy8xTVRMVXFHazV6Rk1S?=
 =?utf-8?B?Zi9ad295Q3dBK1o1Q0NZVklNV0hyYmJIY0piWHZ5S053c1MrK3Yxb2NYZTl6?=
 =?utf-8?B?SUVwRjhheHV6MXFTdEtOZXh0OC9uaFFVTXRyR28rRlV2MkdkeDFVRXFFM3hr?=
 =?utf-8?B?SjBaeGJ0MkpycCtaSlV3YklKNmFydkpvWjByR3p4encyVHZuNzdsdmJoWUVl?=
 =?utf-8?B?Y29HVEt0eEZmK1JyWXI3dHc4bm5uczhWWkhKMmh6YVMvdko0MVRrUHdNd2pF?=
 =?utf-8?B?SFppTWhYVVVITW5mMWxNTXNoVnJvMWgrbTZ6UHVIRlMwb0dSTjl2eXFJVHBS?=
 =?utf-8?B?NytLVStMV0NjbFlGV2FkZE5nMkVVNk9oK05YR2h0VmVEcmlsNmtSaFJ4OVMx?=
 =?utf-8?B?MUYycmVqN3VmM1ZjUCtYTk12MTZFb3I4ZlB1SHJ5bUp0VjhFMHo4K1AxS0Fo?=
 =?utf-8?B?dW9Ld29scVIrU3BxNTFOMWhOVnRkeXhXL3kyVmRYcTlESzBjTmFuUDk0WEpL?=
 =?utf-8?B?ejFDc1VsU1RaeG5OMkhYeUh0S2RqRWxvSDFIb1QvdU9Gam5xN1lCL0F6dGlk?=
 =?utf-8?B?ZmJNamhRVVROdUhZQkRKY0E4RCtrTFl2TVhtMlhRb214M2VBUGhsRS8xRzZB?=
 =?utf-8?B?ZGpxaEFSZTZ0K1hmMlVKVjRlSnJhODhSTlVJTXVTRklaTWt0TkRSdVZCdkt3?=
 =?utf-8?B?S1VrYXdqZTA0YWdoK3AxL1RIR1plY2ttbS9NUGlCNmxiMkxjUzRsaWdldlpj?=
 =?utf-8?B?ci9pZ3RtYzdDRHRMNC9IRnRrdzU3UzdzbTdFR1FycEZuRkdEb2xJd200cndI?=
 =?utf-8?B?cGZsaGJ0LzFSaW9xNWFJOGhaNzh3QUYrUlNscTJCYkFMWVR5TlU3VDBmUExo?=
 =?utf-8?B?RDRXYXNicE9aeVZpcDJLZGdvZVJEdE1oSmplWEFjUGo2TEpEM2xhWGRrN2V6?=
 =?utf-8?B?eU5BZytRYUJKRVMxUHkzOWZnVjdkZUJjVmo4TzZoNUtBeWQzd2xFK2lCcEtD?=
 =?utf-8?B?M2RjL0g4THoraWMvTHVLQ1h3cHU2VWp5SXRNK0J0OUdZdkpqdlozcG1vNE5o?=
 =?utf-8?B?Z1lCZWRtK056QkE1ZWFGTnY4TUNsZ2hjWlRDeFJwN0JVMUpiTFB0Lyt6bDJS?=
 =?utf-8?B?K0dDZGJwa1lETDhLRW40aUg5UlpaS0NOZnczaTFCbVY3SmZVMmwwNmNUNHZW?=
 =?utf-8?B?bE8yR2hSa2p6QVdsVGpKcnNGRDRsc2tsMGtwN0pxVHpTRGhtcnBwYkszTDFG?=
 =?utf-8?B?V3I3OVl3dUNOZndTUThLMzhaeXdubG5qYXZuLzR0VDFCRFc2elh1c0NlczhD?=
 =?utf-8?B?ZHU0WGZrejVKaCt0VDZJSjlMSXRSd2NKT2pINlVFWkVSb2ZjV0hPSnY0MVll?=
 =?utf-8?B?a216NUt4TXkrUHhNelZacWpnSVpNSERQdEZsSlowWWEzd0xkSGd5NkZGbDQ3?=
 =?utf-8?B?UmQwZXhOeWVva3Y3VDczOUxOc2xKcFp1elNZVklJNXRRbVUrMnpQaDNyS1FS?=
 =?utf-8?B?S1BtZFJVU3RFSm5pSVd1YjFnUDJNWGdWRGVjWENYMGhJMkxnQkUwK0U4L1o2?=
 =?utf-8?B?Qkp0bTRDYlVrTXN3SGpqL25jUGtLYjFwT2dUc0VrbFY5N1V2OU9sMUx1UVYv?=
 =?utf-8?B?RGxjYlhKZjNQaUhZNXNHNnJBRUVmNUc5MFYwODFlWklBTTVQczVxY29pQS80?=
 =?utf-8?B?TitQc0dCbmIwY2llZTV2OThpREorcXlLZWNmR0VGN0dLb2xZOTcxdkdoeHNs?=
 =?utf-8?B?T3hBWVRORGk0MDUvb0UxNnQyWk1TczNGSS9Nc3BvbDI3OHpMRk5rR0dpRERX?=
 =?utf-8?B?TEtQSmpUa3lYbWpod05zMGdyR291VFhWTnNKcldlWTNQR2tMQ1JpUkFaZGRi?=
 =?utf-8?B?UXZ3c3hYRDd1VlFtWHRoM1N1ODc3OW12RjBHZnVwTi80NTdsTUxDREw1bUFt?=
 =?utf-8?B?bmZubUJJMm4zY1plNXNHWGxJMWtidVRzWkgxSU9wQjVLdWw0SG1zN3dHSDVL?=
 =?utf-8?B?d0xqS2RqVHIrYkxaWk5BYWJLaWg3UnBJRUVaN0NKYlFIM1hZcTZDbVB5NVUz?=
 =?utf-8?B?dlE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 04N+fU5ueZqHNiGgeOTP0qSDvXrsYQw1GgSpWK/WTGo0U61sFjvqMP1BPBYpt658u3n9Dhs4C8jchCxtmC1U1Tf80MyN9pGQi8TNbeLOJPOixVN5VIw2u2y6EIfYM96jZntzp3p3UQ7o6TQ1nb87wHd16aiT22X2qAbVfzG76Law9uG/A0qtFaGlW5QQmtjdYede/xE4JPwJzR1Yp1E34oCg2k6wgonkxyv4+wgrOlV2qra0mWFjqnHVa01Fca7uhM+EvXUxsGglxmEVlNtbw8/nvqimq/BukdTIsN0NDQxZpMLwEQdmAEl6Lbd2YqRX6eu/krjqnyIBzuC/Vo510VSguKyb3Q/PqCHGgX3RdfAhVG1C0Rn7YpsgexQ3xnEjOzx+n3hjvl8ZGDvkRebQRG7Y5fiAlq4X6gCrO4vAN5EVt2OdtWD81fxH9F6+YYbMi/y7ZiOotOpe2imkJpqcYI3aEup07zg2BA36uB/1XPzuW/DdJoQxW6mAUCe4M2fI4dvPcGhe9eCF/AgAI4x0Sss9TOzSdtkdAFJ9FSfG+xHqFJTyuPaTxYcSCf9UULmcsxBFuhdtsPMsfWrCGRQeJUKTPG5Fja+g+n0aWHvVO2/6KbG9D/XaTXLcA4/LY0w6l9gtnbPFa5/NhKEaZGvFwXoW8Hox71rbKYmo0tBZ3BYnqoAkN7xeLUt3ieBFIOQgqvACwf9/yTNiam2RS3dGnj3qdC+SbF6r2xdOSfQd8xJZYQmeimXITjMAT0+6EJ58iODcPAMreT5qrRU2kVG0diKARecaBblgUbNBI8ioWQwrOvT8nOuIkTgbofZ/FojmibWdNpgF6uLnjeRXqVeq+VokjffMozisd1s8+2vv4oU=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 46ab94eb-571a-47b7-d589-08dbd9593a9e
X-MS-Exchange-CrossTenant-AuthSource: CY8PR10MB7243.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Oct 2023 15:02:26.3384
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: jFDukCzHoNxOa4i0WKtYBWMQ+/KA3sbarOOEWawaFtIppsOTkCZ7NwGy/vONgBBZhIV5IdwsLF8FrQdsvpG85Hr8LmjW4T+PAw/+zEAH9Vs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB7364
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-30_10,2023-10-27_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0 mlxscore=0
 spamscore=0 phishscore=0 adultscore=0 mlxlogscore=999 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2310240000
 definitions=main-2310300115
X-Proofpoint-GUID: yceGmQaDfHBXOK7Wp98uVWDhInGY5IqT
X-Proofpoint-ORIG-GUID: yceGmQaDfHBXOK7Wp98uVWDhInGY5IqT
X-Spam-Status: No, score=-5.3 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Mon, 30 Oct 2023 08:03:23 -0700 (PDT)

On 10/30/23 3:13 AM, Shigeru Yoshida wrote:
> On Wed, 31 May 2023 15:44:17 -0700, Bart Van Assche wrote:
>> On 5/31/23 09:43, Shigeru Yoshida wrote:
>>> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
>>> index 12869e6d4ebd..86b43c069a44 100644
>>> --- a/drivers/scsi/sr.c
>>> +++ b/drivers/scsi/sr.c
>>> @@ -177,10 +177,13 @@ static unsigned int sr_get_events(struct
>>> scsi_device *sdev)
>>>     	result = scsi_execute_cmd(sdev, cmd, REQ_OP_DRV_IN, buf, sizeof(buf),
>>>   				  SR_TIMEOUT, MAX_RETRIES, &exec_args);
>>> +	if (result)
>>> +		return 0;
>>> +
>>>   	if (scsi_sense_valid(&sshdr) && sshdr.sense_key == UNIT_ATTENTION)
>>>   		return DISK_EVENT_MEDIA_CHANGE;
>>>   -	if (result || be16_to_cpu(eh->data_len) < sizeof(*med))
>>> +	if (be16_to_cpu(eh->data_len) < sizeof(*med))
>>>   		return 0;
>>
>> I think this change is wrong because it introduces an unintended
>> behavior
>> change. A better solution is probably to zero-initialize sshdr before
>> scsi_execute_cmd() is called.
> 
> Hi Bart,
> 
> I'm very sorry for the very late response, and thank you so much for
> your feedback. I'll prepare the v2 patch as you suggested.

We actually just did this patch for the next kernel where we just
check result before accessing the sshdr:

https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=6.7/scsi-staging&id=f7d7129c6c24168b9be7709b0b37767b5f743cf3