Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9d356278-c826-dacf-cbe0-79f512b7970e@oracle.com>
Date:   Wed, 17 May 2023 16:05:38 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] scsi: Let scsi_execute_cmd() mark args->sshdr as invalid
To:     Juergen Gross <jgross@suse.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>
Cc:     linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
        "James E.J. Bottomley" <jejb@linux.ibm.com>, stable@vger.kernel.org
References: <20230511123432.5793-1-jgross@suse.com>
 <yq1ttwbsoii.fsf@ca-mkp.ca.oracle.com>
 <6614f626-d174-03d0-0993-79e6f6169b71@suse.com>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <6614f626-d174-03d0-0993-79e6f6169b71@suse.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dTd6MHR2cEtlejhOTU44U2Eyay93b1pRb2ZWSDBoRCtQM2JLc0RqQSsyRU0w?=
 =?utf-8?B?R055cWdTOURmbi9MbEdCTUpveXp0aTdQT3JGanhLMGNkWkdwOTduUFRacnZk?=
 =?utf-8?B?bkhTQm5xY3Z5ZnFxWkRKY0VFU1M5ejYrbXUzcU4xVW5NRUcvWFEvQ0EydzMw?=
 =?utf-8?B?cFg3Q1ZJYnYrQ0grS2dTR3ZhTHJkblFzZmwrNmR1MnJuVlUwcVRkbVVEQWoz?=
 =?utf-8?B?OGt6UkpqNm5xQmFEbU5Mek4vNmpNVU1vM2lWUlhNT3ZtQ0NRRWlMakk2Uk5j?=
 =?utf-8?B?ckRkbXgvbWlOdFRiMmV6bHpZRWpMVTdJZzl4UDVhV3pyMTZLSW5LMFZhMUFU?=
 =?utf-8?B?eWZLSGh0d2dHaHBYN2c5eFI0SGFOQUVpUStLc2MvVm5OYnRSVVhWV2VWem1r?=
 =?utf-8?B?dVpWNUkyRVAwSWFXcUU5NVBEaUhSYm51NFc5d1VkZFFaYkE3Qjg0NTFMb0JQ?=
 =?utf-8?B?eE5hemVwdlNQVDUzU2R5TXNrc3RPTHVxckI2elFvVVdES09YRlZRUlZNN0VJ?=
 =?utf-8?B?SGlXbk42Q0wwV1d2SXJwc0ZOSjhzajRSRWU4NGNiajhTa3UvZ09laHJJV3Qv?=
 =?utf-8?B?cHFDQmpYOUtsVkZjZEd5WGVlSzk0ejczSy91L0xQUTNiY3RXMmRCWWV3Z0dV?=
 =?utf-8?B?L3JOZHMvcnJHMldUczZYOHNadGV3T1lWSmZpc3hRNXpsS3dYR3VZR3lFQmFn?=
 =?utf-8?B?dUY4YjJrZXpqZngwdnZXMXRRSVFodkFJZDBJV2dGUzA3NkpHc3lQV3QyN2VB?=
 =?utf-8?B?MjZLaXVnWUpDU3p2UnBRblRwSU1tdWgzRUJpM0VTSDhPaVBJK0ltV2I2UXJ0?=
 =?utf-8?B?MVkveGpRYWlGaG5XVUpDaEtvV0dIc245Yk1MUWdhdkg5enczNFpBVTdDYVBR?=
 =?utf-8?B?NDlTc3BHT2FSeHVoL3dnZ3NtUi9XbXVzTjRhdUVnc1RXUFY5ZUdmc29WL0Rr?=
 =?utf-8?B?ZVVxNHd6S3laWnB3K01NNVZVYVE1aExUV2c4WWZpUWtjVjEvODNQeWx2dm5y?=
 =?utf-8?B?cmV4Ukx6cG1HS096eFV0eGRrZDZRaVNvQ24rQkhzWERRSWRrWXNMbDBONXp1?=
 =?utf-8?B?eHZXa1BmeFo2eVJMcnpWUEhKOVFkdXl2TVNhL2FHRFRDWHRIUDkwRm04dzVP?=
 =?utf-8?B?a3NqZTgyTFVUQnpJWDNidFZGUXlXaE9SM3IyQnhocnpBTEloVlhHcngwVGFZ?=
 =?utf-8?B?b3NCNnVOajh1MjFrcVdpQVFya29SZ0pzejNFRHhLR1BoOTR3R3EwMWJjMTJa?=
 =?utf-8?B?QjZ2VlZmUHZldmtPQWVmZUg1RklOWHhFN1NOclNZZmU0SnI3MHk0K3VndFEw?=
 =?utf-8?B?OTh5dU9TdWtkQ09KSWRjY2ZaM3h3RnNLWUxDNmZNMjZvTFhrcWpRM2VtZ0Np?=
 =?utf-8?B?VWgrblJzY0lPWS9hZTFoV2JUMjc5QTk5c2d5V0FkL04reFRLVW54SUorNS9D?=
 =?utf-8?B?bWRHZXVncGV3Wi91aDJUOUgyNWdxaGV2bWpsU01MMGdTUlNWYUpJdmFyTmVQ?=
 =?utf-8?B?cXhFeWJ3WVE0OTBtMFdudmVKNzcwUUYydDQ2NUUzMk9QazRmZUliTjZlbTdI?=
 =?utf-8?B?OVJac0k4TVV3OGRBQTlZL1Z5eGVtNHgvWXRSWlNiOTUxc24xVFFKY3dPZzA1?=
 =?utf-8?B?Z0J1TzA5M2pSemY5UXg2TlZqUnZLNFFKT0Rzb2tIaEdTWGtDb0l6NGYweHlZ?=
 =?utf-8?B?U0xOT1NUMXRwSFdKOGxKb3dVMnh2SklSR2tydi9KdzRwWkZCRXNLR1Zrb0Zp?=
 =?utf-8?B?MzNQMXdNb2ZsYmZVMnlkbEYrVWl6MFQwMmNTQmJPcFdXUWdpT0V4a2djazdN?=
 =?utf-8?B?TFBycjlkTkx4QXI4VlNrSXZHVTNsc1d5SVN5US96SVEwUGZtTEszaUVzOE1D?=
 =?utf-8?B?alBlT3ppT2h0b0h0K0pEcEFRWkgvMmljeHI4M29DR3BRa1lQb0xUUU9VcFl3?=
 =?utf-8?B?ay9BQjdOaUJmbXV3ekdCeSt6ZmtYUnQ4cE96TGxxZTloTEVhb3NUditib0do?=
 =?utf-8?B?SmFMMzNlRkdWb2ZJTnJSNk1DUTB2ZG1mS0VVdGU1SDJKYUtlNi9aZzRxUzJE?=
 =?utf-8?B?R2FGVG5VKzZGcE9KOHpWUkpwYjMxY3REaWh0WThPUTlLV0V3QkY4ZlV0SG9R?=
 =?utf-8?B?bDZFcERSbzRuRURyNmMreDZaOU9qbDFQN0lJVWh5S1VCREdyRDZqd1lGbVlQ?=
 =?utf-8?B?Mnc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 5oxUBPOkEwVTjXv6BKj+YgZ21OypOMMnaF+SfY1P1utvhYx+bEq42h5eLjk0AdSxsxyedCsYuPgRVvAF0s83JObNXD3HIrhNV46uof5lds/eHDzAaVdgS07U2PfBfjqQJ0vCcO1VEutjOVA7FxHutXZ1FADgGY4tAb1ntQPpb1zRVL7SuetrZw5e1nad2STs9lxfkdYNjGHYjzgpefY2r5fWvLXSY9x3MIdGQEL08kRIQqjz6qRH7VPCzoUqDEITeR6gvRIDfcyp6s1lz56oxyGkLhMqa44RWsUelLHOKfYnQ1ZvRWrwbBN0XWepbt4LVm6EtwY7tEs7+f54O7S0t2JkADpNMRuWw68bUCvHj2ixaQZpIL3q+mCSoK3Ol5cX5KCcXsY7ThCC3dEHXZ4/KoTMvmvwrvGgppEMiuFtpiihIX3KpHUpCivfqVvvh/eDFfvKDey+A07Ds5fP9OFBpFxbMen7UFtcV5zw85ajZehmerp2ddcZfKKka05CGgSNRtPSCHkEos8heLQxZ6ClbsPQ3NKZtnQwNl97D1TUmv66vgKWCf0NDi9PQFv8abdE9PpLrpitEjiT9Bd0aJzVW7HeTvdLeU71GLG71kjTTXL8j2N69/YRNlfYbsuibqIdpjw6j1jsdx5bjkx5uhH4E8fBNJOJJBSNS1qgBfV8A0tXmNlVCJyrya4AQAuspxs9J9RhQ6+nQnh3+i0NdUw6zqnwzjhZFPoot1PqAWRr/6mJKWt7DA1GmAR6d744w8RED+h85KlxePPjAdp35BKi+ocipMN/BTSS7HoF7pbKkm/YO0CRxLvufblLf2g2Kbre
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c681f9da-23e6-4504-e74a-08db56e82f02
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 May 2023 15:05:42.6672
 (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: bNRh1AMdZjpzPttCUyh5pCRxyo6dTrolsx6/9vNOSkDl5g2VumA94RStw9Aw/LysXQa1rIYHZRgydOqJtwhZrw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4663
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-17_02,2023-05-17_02,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxscore=0 phishscore=0
 mlxlogscore=999 spamscore=0 adultscore=0 malwarescore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000
 definitions=main-2305170121
X-Proofpoint-GUID: ZVznW2E36ppi1L9eVVcAEMjAIwUJt74s
X-Proofpoint-ORIG-GUID: ZVznW2E36ppi1L9eVVcAEMjAIwUJt74s
X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 17/05/2023 05:54, Juergen Gross wrote:
> On 17.05.23 04:06, Martin K. Petersen wrote:
>>
>> Juergen,
>>
>>> Some callers of scsi_execute_cmd() (like e.g. sd_spinup_disk()) are
>>> passing an uninitialized struct sshdr and don't look at the return
>>> value of scsi_execute_cmd() before looking at the contents of that
>>> struct.
>>
>> Which callers? sd_spinup_disk() appears to do the right thing...
>>
> 
> Not really. It is calling media_not_present() directly after the call of
> scsi_execute_cmd() without checking the result. 

Is there a reason that callers of scsi_execute_cmd() are not always 
checking the result for a negative error code (before examining the buffer)?

> media_not_present() is 
> looking
> at sshdr, which is uninitialized in case of an early error in
> scsi_execute_cmd(). The same applies to read_capacity_1[06]().
> 
> scsi_test_unit_ready() and scsi_report_lun_scan() have the problem, too.
> 
> Do I need to find other examples?

Thanks,
John