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: <554bfa20-2228-8655-09e2-492cbfa183fa@oracle.com>
Date:   Fri, 19 May 2023 17:06:03 +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:     Bart Van Assche <bvanassche@acm.org>,
        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>
 <9d356278-c826-dacf-cbe0-79f512b7970e@oracle.com>
 <60aeffe4-b31d-4ea3-d4ea-f50ae25e0316@suse.com>
 <74879c87-689f-6a8e-a177-8bde4c9c4e51@oracle.com>
 <fb0efbd1-a54f-09d6-bd27-6f665b461e58@acm.org>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <fb0efbd1-a54f-09d6-bd27-6f665b461e58@acm.org>
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?RW92Z1NtWkYzV1MvZDdGbXQxeDAxY3lZdzJBVnZ4Y0x2UGhlL09YeGFiYmYv?=
 =?utf-8?B?aFY3YitmQXFrS0RDNFdGRXFoWmNDTEIrcHh3V1NKVzJZb3E5NUFrM25wNExK?=
 =?utf-8?B?REVIaUpKOEFENnlyUW1BT3Bpd2NreTR6TWJFTGR3U0kxQmZmV2g2UTNReGNF?=
 =?utf-8?B?Q2R0bG5QRDZ5cnNSN0RqaWRiVjRLL0hFaUJkaFBXL2IzUnlpSWJ6UTd3OEIz?=
 =?utf-8?B?SU85SkcwM0t2Z2QzV3ZFRTNiT2dITjRZWVV4QmYxcXJXeHpoc2lsbjNaUjRp?=
 =?utf-8?B?SEx1emdGcWsrajVvUGxnWnVnTTJSOFV3MEw1djZNd1l6eVk3MENoUWlyYmVS?=
 =?utf-8?B?cmxtRHN5T0psVXZBb0dDTXVidlpsOGlPbzZoQ0hLZk8wK1BwY2xTRXRTaldI?=
 =?utf-8?B?Ykx6R092VTVQTWI3VGRFQW9yTStadXdyVVVqY3NqSzNwWXF4ZkUva3FTNXJh?=
 =?utf-8?B?MFNodXA4Z0pkbGhpeUFUbWRwdDZZSXUzNXdNcDRKeHB2em90ck5tOS9SSmts?=
 =?utf-8?B?M2FyN2VqN0lCTEMrbzgrK09tcVVselFhZlhyOGUrSEY3a1ZDanVIdlJaSEx3?=
 =?utf-8?B?TCtXK0NaVlM0aVhMbVZzTFN6cmc2NVBDckszMzVZd1NFWWNrVlI1dk85M0x3?=
 =?utf-8?B?U0FzL1FRUU5mN2dFV2ltUFhScVY5QTFSNG90RngxZlBNSWloTkNLdHlENDJS?=
 =?utf-8?B?VkdHOGdoVjZhUHFjZ0JlMWFZTEZiL1VhQWtHWGVLTkNFRnI1TDEzSkMxS1Ni?=
 =?utf-8?B?eG9sYVExaitBWVE4L3FTY2dzMGtKOFRYdWlwK0lqbkQwYzF0QjMxdkgvbGhJ?=
 =?utf-8?B?RXFDMmQzTlIveTljWkJHbktXZDgrMnJadm4rVTBUdVhOZHpkVTZUZFRxczVC?=
 =?utf-8?B?TjVHRXlnV3BmaTB0TytOL2VReHA2L3VzMGYzRytCK29ZU29FbEpsMHdxSDM0?=
 =?utf-8?B?RVlUZmx5VEJMeHJ5ZjdaRGN1REpIUzVUb3RWMmEwUldJZXEzMjQrMjM5SEZP?=
 =?utf-8?B?ZG1pY0V2VmZSVkFDbjZUd1ZoeHVndmZnSGtsTFpncEs0M3hBV2xHbEhNWS9w?=
 =?utf-8?B?Vy9Sd2hlYmRVeEhsMjd3dnRudUJDWU04c3M0TmVFVGtHT01pb2ptU0t2MTlF?=
 =?utf-8?B?OGR0ZjViaDJCaC96cWQwOEJTVm9nVC95dTVGanRLNWtXalo4dDIwVkdQb0JT?=
 =?utf-8?B?eVlyeXFWRUlxK0h5QVNDTUdsZDdZeUdHTXk3cmFlbFVOUjdZUkFXejJXU1dh?=
 =?utf-8?B?cEowYjVVaUZIWDBQNlMrWEZLWXR0UnZaTzFOeDd5WEliWmYwbmpVRFNHdFVK?=
 =?utf-8?B?R0VWUFJYKy9JeFpRMzIrK1FPY2lOOHYzNHYwWUs1WXArRENRN1BpTTVQQmVv?=
 =?utf-8?B?eTg0R0o4OVNta0VCYjNwNmJSTmh1cExYQW1qRFQrQXg4aXhmVkg4OTVmd1cv?=
 =?utf-8?B?bXRIVitpZ3JMYWFrS1BKV0xjU25EVWM2NnZJa0czOUtmR0ttMzlJVWRvV1hk?=
 =?utf-8?B?Ly9XazUvZnZxQ01OUG8zWjM4ODdGN3poYm1FdWxjZlBiZ09NR2FDSkk1V3JT?=
 =?utf-8?B?RERWS1NJcG1RaTVkTHMzYm1xNk81Q0xmMmcyVEhESmVkZU5URGpFaVVKZGI1?=
 =?utf-8?B?NTI1ck5HbiswT1h4cEFvQ1lOa0NzVmhKbDdLdDVIZk9XLzg0NkF2RnRHRG43?=
 =?utf-8?B?YitCTVRoUHFCRGJjOGl5K2Zxd3FRcVhKdUFIQThZWEk1UEY2elZ0Q2MzbXFr?=
 =?utf-8?B?OHhqTDRDQVBIWFRCdFduRDc4Y0R5NlhsUFJZMllPemVkeWVXNjdudjJyUFVB?=
 =?utf-8?B?T1BVcUhVTnZ0YnpqeUFwRHBXZzIvRGhmMDZCZEJvY3FyVHl6QThmSThUZ2Fl?=
 =?utf-8?B?NXFzcnZ4WC90SnJEWXdLQitQZmZNOFZGYmlDYk41SldyT1pUN2VVbHlVb1c0?=
 =?utf-8?B?dzNvRkhrRTNjMXBsZHMxajhIQmorWmdzVlBlZWdaRzVSakJVWlpuQlV2RFNM?=
 =?utf-8?B?VkJyVVdTOFRlMkxXbXgrUnFZeERkSDBDN1d5YUQxVmZGYWNvd2w0aFNoRVJT?=
 =?utf-8?B?ZUhmaVd6dUFwNWhSVC9IU0lkOGRLWVB1K3lNUWQwMmdGUGt1QjJTMXJMMEdW?=
 =?utf-8?Q?Qb+DoHKyHf+2/iehVqDri1tQX?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: X6q/bqbbSNTrQrZHCH77kIDwrWpbI5tXtK+SPqjfb/yPUY9qn0HU0uj9JxNd3SYVDt/CiPQPW8XgWWpPgKYd+EZi/4vJ6zg87EAeVTK0Yo2Trl8Oc6f70mShy40sHPJn+tsY6vFUyxi8XB9dPhyhCwBp3zqoXJnFdVw97bxu4ZEeow8QDMITH4w52Cn3Jmlq24a7nDLaL+4AMNnuQOf2CeI0EVFhCrSlasGBG9vDxhgt/bbkz8zyW3bEFExlxuh14OWcIcF/FgV50uTCKxOte7IwXGEJUqE3l9EZOPkIzFszW5Rrfl2TBE45ZU6l4kSMsqIdhvqK7Hod++0HhdbdcuIabJTaCuaiOdBQOzpL7q1xXTOdvIgfkhkj6JaeJZwAO9IGzoB2ctvtIv4xf1tW+f2Yg8rpy9qm/9Z8O2R6MnfltE9xdoT+7KtR+Rp+KxFP5sPrQ0TtdyVBqCR5j3/AtzT/GR9PzStLFhnRreA+HughHBg3ZmIAIz+5c3sUEpsmCn2w4+7j2UuRIt4qXF0PZUefdu3vAksxkOYhFBWAVDzwLgNGuxlnRnUwmZ2i+T4MNmYrmtIEWFXcR11Ww0MX5oFSK4tDakHPFzyvbn+/qm0hMBljDuaTZvop1Ibm/VGAjAP6x2zpb2A83tw4nO4f5sE//MvSqAUfU1OUUSiUUqIes8FFQ9J67ZSZtToGk3Xsx8C3KWliXeI4LBYiAXiXjMnFTLIh0Wb5cKfbgASNIU7PRRIkjwuyVX9qeIyFTINoWyAFBNIJvOSN/jbvzCNzL1XuJL4KP9cEptbTx1V1C3q+C4l9xesGWAo+thh68dKF4tkEBzu+2YItMGGz93D9NA==
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2d75bbde-7843-4290-a271-08db5882f4d2
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 May 2023 16:06:08.1768
 (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: 36EMGQ2h04D6uLkDQudhKowpiB1Vdy2kgklz02KQ5gMcS1gV5ta1/R+9vGglndokSZrv1zT5b9VlCGiSNOxkeQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5642
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-19_11,2023-05-17_02,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 adultscore=0 bulkscore=0
 mlxscore=0 phishscore=0 malwarescore=0 spamscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000
 definitions=main-2305190137
X-Proofpoint-GUID: 3P83intpm1Y_yROOjAaqeM9lFkywRMMe
X-Proofpoint-ORIG-GUID: 3P83intpm1Y_yROOjAaqeM9lFkywRMMe
X-Spam-Status: No, score=-4.3 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 18/05/2023 20:54, Bart Van Assche wrote:
> 
>> Further to that, I dislike how we pass a pointer to this local sshdr 
>> structure. I would prefer if scsi_execute_cmd() could kmalloc() the 
>> mem for these buffers and the callers could handle free'ing them - I 
>> can put together a patch for that, to see what people think.
> 
> sizeof(struct scsi_sense_hdr) = 8. Using kmalloc() to allocate an eight 
> byte data structure sounds like overkill to me. Additionally, making 
> scsi_execute_cmd() allocate struct scsi_sense_hdr and letting the 
> callers free that data structure will make it harder to review whether 
> or not any memory leaks are triggered. No such review is necessary if 
> the scsi_execute_cmd() caller allocates that data structure on the stack.

Sure, what I describe is ideal, but I still just dislike passing both 
sensebuf and hdr into scsi_execute_cmd(). The semantics of how 
scsi_execute_cmd() treats them is vague.

Thanks,
John