Message-ID: <e4b04313-20e9-0060-5e29-17fac62a8234@oracle.com>
Date:   Tue, 31 Jan 2023 09:02:07 +0000
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH] scsi: aacraid: Allocate cmd_priv with scsicmd
To:     Vegard Nossum <vegard.nossum@oracle.com>,
        Kees Cook <keescook@chromium.org>,
        Bart Van Assche <bvanassche@acm.org>
Cc:     Hannes Reinecke <hare@suse.de>,
        Himanshu Madhani <himanshu.madhani@oracle.com>,
        Adaptec OEM Raid Solutions <aacraid@microsemi.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        linux-scsi@vger.kernel.org, stable@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
References: <20230128000409.never.976-kees@kernel.org>
 <1bb8de35-16ba-6c7c-0ef6-67a7226a0a2d@oracle.com>
Content-Language: en-US
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <1bb8de35-16ba-6c7c-0ef6-67a7226a0a2d@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MUJQYnJ1QlB6dGJ3dFBXUXNiWlpwOTczTXFSNlBUeTVJRTF4UlNpUGJEV3hW?=
 =?utf-8?B?L29XOGdzakIrZjdyalJvVlJkYU81dWk0bHdNKzdDbXd2UGY3cVl1azFBUU5m?=
 =?utf-8?B?cFNueGZhdDZlOHJtZHBiSlBuM0ord2ZRL0t1Zmo3V2VIeUhkS2NmbUtJVHIw?=
 =?utf-8?B?QnBTZ2ZCZVlCNUxuaHpBMnAvSjFzeFRpZ2hpbnpYZlRmRFpRS1Erb25HcWpw?=
 =?utf-8?B?QitEdG1iNkVJRXVxUTdGeFJZOE80QmQ4ckRyWThGUHJXOG9sdFBDclVlYlBv?=
 =?utf-8?B?RkhvbFQxeTcxVy9GdXJyaEJya2orZDl0Zi9sZGRyaUhsdnJKRzA0THdQQXZW?=
 =?utf-8?B?YStYOVFxbFkxOWlMYnZ6UDFLU1NQZ2NOaDF5ZmtnQVZnWkcxVDRpZVA3MnQ4?=
 =?utf-8?B?UndqK2YxVDN4amtwNm9WTXpjSnlmcFFDeGM3elprVk1zVEp0eFBjOHdQdXlx?=
 =?utf-8?B?bDVHSkZ6Slk2bHluUHA4eHprWU1qMVJGSFpxT2swdE96YkYxQnIwWVA5SDVq?=
 =?utf-8?B?ZjlRRDU0Rkd2MlNpa3k4RkIrRXQ0cnFmUWw0Nng1TlpYcXZMSzhOT0dJS3hn?=
 =?utf-8?B?Q29SSEFqUEtqQVBrdkRtN1MyRXJQWk1DcnY4cDhxS0liMTB4VHBjbk9wK3lX?=
 =?utf-8?B?aHNOTlFVKzJtZkE4MWh1czJyWkFka2ZmSEtGaU81Zlc0N3FvMlYwWStUV2hZ?=
 =?utf-8?B?b0NLbmVDa0lJdnVCMUk3WlVHdlk1L2RqTVgvZkFNLytPbUs4bnhqbUd1MThP?=
 =?utf-8?B?TzRRMU5nUW94bXloOUlVQXJhNlNtMVkvL3J4d3NRY3E0bExqUGlSa21ZU09M?=
 =?utf-8?B?T2FGR2c2VEoxZEVqZmlDWWRwb0pwc1k1ODR4ZkNjTll6alE3cUMra1NmVXk2?=
 =?utf-8?B?LzFxK0hrUzRjN0dkZldsS2ZyamtGZDl0cHJQeTdGejRIdi9HSlRCMGxOL1dF?=
 =?utf-8?B?SnBNRE5LTXRYZDRrellnVHJWNTFLdG1maHZ5ZWdOK1E2RmRrV0VUUmhXa0FH?=
 =?utf-8?B?VzFoRmhwTms1KzlmVkVVTmFHUUpESEtmaWZkSjk1bVd6VVdLdFVGTTdra1Y1?=
 =?utf-8?B?S05TMHhBN21DNmV4UjNzUEVTUHVpU2xxZjl4b3YvdHcySlozWjN6d2tmREcr?=
 =?utf-8?B?RzVaVVZmbkhOVmVUYmxtZ2ExK1RhbWxiOVV1QTVwekRUYXZIUTh3SUFGeVBC?=
 =?utf-8?B?dGFOWWxqR24ycEZ6Ri9meEx5alIvb2hFbW9xK0VFVitQQ0NzOW1VcjRuRDVC?=
 =?utf-8?B?cWNvMG9EditWREdkZFdzODVtRE9uU0FoL3IrOXJVbGlIVDhpT2NieHprNXhO?=
 =?utf-8?B?b2dUc0VFOXF1eGR4L012Ny9qZk1mb0dNa1dNQnZYRXNEQkVBUzZycUJVa2Nq?=
 =?utf-8?B?VWkxTTF4VW9KRElRd244bmNWZHMwNWNYWWxZeTA0QS9KTHdNMHYvUVg2eGNQ?=
 =?utf-8?B?dUEyOFZXUTRhYmtIZGxXRW4xNjNJRHJMYjh1YUJuc2IrQlRwUXZkellqejRq?=
 =?utf-8?B?TUZzaDZ1dE41TCs2eEtoSWZzQTZpSmRtNkxUSmZtbUpWMGNJYUNDYnVqZHRh?=
 =?utf-8?B?K2pCNVBDalpvQTVDVVVNSDIxcXdwcDN5eStRaHM3aGkwUDBoY0kwY3dOV0xq?=
 =?utf-8?B?UFZ2NUpzS1BDWVB4dTZJaWRySWFjYk5hbDBuN0wvTldaV1FubmRtOWFvRkdN?=
 =?utf-8?B?a1JCbElMdUovRUZGam9vUW8zSllQMDI4enZoT3dMNGRMTE5MRUtEZmVubE50?=
 =?utf-8?B?Tk9kRy9UdStvbG5STVlkekdrSmQ2S3dyMmpuRFdPT1RiL3ZoQllLY2N1U2Ex?=
 =?utf-8?B?c3VHRERRdis1djN1bFA1VnE1QTlPNHppNGxXeHI4Rlh1bzJtV3ZMd3l5OVFI?=
 =?utf-8?B?aVkxbG5KNjJkdG9iZVVFMFJoWjk1bDdYWDRIY1ZGWGJ1NUd5V0pyekxmcmtx?=
 =?utf-8?B?aFhteGNuU2hXK3JXRm4yMjVYL1lpSloxYzlHTXU1OXpFckxmRmM0TFB2bHJl?=
 =?utf-8?B?Z0w5ZU9Gd3RXTTAwUE9FcS9mY0o3SWdIdmplYUdsbFZrb2RONXIweUVoNC9a?=
 =?utf-8?B?c1Nka0FzTVVWVlc2MTJ5Z2VhTkhaY3lSQVRWWlI5WldaR2tGcTJISUlOK3lM?=
 =?utf-8?B?ZlZGUXB5U2QwVDNqajFYTzRuakZGeUt1OWhnNGFSaU9CYVBUM2E0ejJsaGZN?=
 =?utf-8?B?RFE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: xOlp7jZROK9+6GmmMcNLnRiwR5MD+tYo82jBHLAZ/89uBz7+MD3772zLdzjMjd+ljLQqXgTgo88LRzAFP2rUXsug5slrOM79t9K7OWGhvwLYIPcYSj5LKOVdErBFZQLnoFB3HyJF2/D5ak6yJHd6HBCQHmL1L7fRQn3vTTITNCn0a85J4LS/eQKIErrCP4KLhM9oN8x/toheHhdEqS1Q0V7aiJjwG7RQcYPuSNtXDX05RGdt8ktshBUC7qxj9ako+W5gyWpm0hySpPsW+0MYkM0zNQ72RqQ8DRWNpAV79dgaHFWHxSEjmK382QcwCUBEXpgd6eKWqDDeMgC9tESy2G43lRCAtK0jsBiCoL3l27p+jQCkJN9RKlOdTMmizW2fH3jshPZ9lsK1/p9pHa9JisYbuxuBk2Z/uu0uYm8D8k+ORxpKvqR8rL7DPcIa43olZunoFPxBBUA8LU+K2f1axNo+22BJzQDmS6NEn5ns4zP8gLxJd6F9uv3MPnfVsQxpKwIxT0u5b0EdPGOVvpcpelmv94Tbttju6JXUz8j+Ke9RMdRrcjMUEmp+0IGWwscPRfXrVvv14+3K7E5mD4hTrjP8frSeNGQmTDYW02pFHQBVwCYy+PmPGSrMfDXUCV2uU2+ip6CMgqUz0UBMj2n217zxT9DTYK4wiO6W3aNX2xFEtwODNw6KcHRRP7GoLvPOUBllfUSWH2t2Aus7ZSIFfPxKxSmRbf78r3ncNzygUHd+U/PaxgOEY4oXWzh0vx4U/HFoS+wJvmTLnVj1i3nK0HhgXp25ulK1UVq3IVhCBzsh9HSzVsGLffD0QxFuTp5XGpjwbXS0XXoZbPiTjXMpsyBYdp9T9Z1vK9dM0WB/AM8tcLN9bA6Pq6vlmTlgAWec2v7LAHtntSERBttLp11q0uDqBrUImDjDH8TMt6+r0mA=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 49441b1c-0f00-4469-e9c8-08db0369d791
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2023 09:02:12.9451
 (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: CuPalvBdrg/n5MtV3XRy9/fTJjEJixolLVdkI6unY+y9eL2hrw2iXrqC782I5mFbK9TZN6odTcnDFg5fLmBQFg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4195
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-01-31_03,2023-01-30_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 phishscore=0
 malwarescore=0 mlxscore=0 adultscore=0 suspectscore=0 spamscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2212070000 definitions=main-2301310079
X-Proofpoint-ORIG-GUID: W4O5eK-seLzr491owTUJ5nBILtM88RKf
X-Proofpoint-GUID: W4O5eK-seLzr491owTUJ5nBILtM88RKf
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 28/01/2023 18:40, Vegard Nossum wrote:
> aac_priv() uses scsi_cmd_priv() which has the comment:
> 
> /*
>   * Return the driver private allocation behind the command.
>   * Only works if cmd_size is set in the host template.
>   */
> 
> This is set for this driver:
> 
> static struct scsi_host_template aac_driver_template = {
> [...]
>     .cmd_size                       = sizeof(struct aac_cmd_priv),
> 
> I looked around to see if there was some kind of "allocate cmd" helper,
> but couldn't find it -- scsi_ioctl_reset() allocates one (together with
> struct request) and there are a few uses of ->cmd_size in
> drivers/scsi/scsi_lib.c but there doesn't seem to be a common code path
> for this.
> 
> I guess you could use dev->host->hostt->cmd_size or something, but that
> doesn't seem worth it since this is driver specific and we already know
> what the correct value should be.

How this driver allocates a SCSI cmd in this fashion is not proper, and 
hostt->cmd_size would only apply when the SCSI command is allocated in 
the proper fashion, that being as a request - __scsi_execute() -> 
scsi_alloc_request() being an example.

Hannes did have a conversion for this driver to allocate a request in
https://urldefense.com/v3/__https://lore.kernel.org/linux-scsi/8efc0e24-3000-39d9-7676-e0896145f247@suse.de/__;!!ACWV5N9M2RV99hQ!MealB8BN3q8cxYSaB7yKEbHyDmFTNl0YNVQXpVw8Zd0-iNqQ-k4IFxnqONpixfavb0DqGWnkbDVjBJCE22mYq5Ly8Xs$ 
- hopefully we can progress that work at some stage.

Thanks,
John