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;
To:     John Garry <john.g.garry@oracle.com>
Cc:     Juergen Gross <jgross@suse.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
        "James E.J. Bottomley" <jejb@linux.ibm.com>, stable@vger.kernel.org
Subject: Re: [PATCH] scsi: Let scsi_execute_cmd() mark args->sshdr as invalid
From:   "Martin K. Petersen" <martin.petersen@oracle.com>
Organization: Oracle Corporation
Message-ID: <yq1v8gmpkq4.fsf@ca-mkp.ca.oracle.com>
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>
Date:   Sat, 20 May 2023 21:19:43 -0400
In-Reply-To: <74879c87-689f-6a8e-a177-8bde4c9c4e51@oracle.com> (John Garry's
        message of "Thu, 18 May 2023 11:57:36 +0100")
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?HX/6D1avUA7Jghnv+DXk7JOhHFmXQw+Fdxue5SWwjMeHiPpVuJvf6kmZP7V1?=
 =?us-ascii?Q?ycBacpXDslrq5A9DWLFMmZHH3mUlkuwd1A9LHrlz9r703nvOy+ZCFslmFNlp?=
 =?us-ascii?Q?VQgAg3tdjgU5nWBuPiomRiZTAu09ljAPn8OK3f1jtbRwQ7YSYX575PgsQ5O/?=
 =?us-ascii?Q?9Rmw6Zks7Z7XnwcBzYXx0QQ9ecr9gs3qpuBE4SmbLYXidUHfcMOBjwNOcIK+?=
 =?us-ascii?Q?baUkEE9wsn5PM45ihHUlvJnffmI11XKGE1NoG8AuKpt0XdCABOvt9a9HbDvb?=
 =?us-ascii?Q?MOWW2h2SycWc1j9WaX3J0Fy9mHONklIvmAcPjGzUAcWPAstK0sxyLOD35r5b?=
 =?us-ascii?Q?H6Aml4Q3HHoh0K6iqFp5rhbPKwfxFE/Qx1GAl5mKOvpfEq2iNPPtDczb0RFU?=
 =?us-ascii?Q?E00f0r9L/Me9QMU48QQU8HsVNb9uBORtZot7duzqZFTyEtBKLfjxPY/J0Lx0?=
 =?us-ascii?Q?F2EGWcm14wSzcqMboz/u3zjLsdJIZfVhOfAcZAPacmaXtPVKD81f61INaOOl?=
 =?us-ascii?Q?qCKh0/6od9dIKJgKLDcM8XDTn1DUyCo1A9VdufnuKsEKSrENo58u/i7f5Fic?=
 =?us-ascii?Q?V0UtB7a9CR3FLUCxsm2DGzhZbpX0/kWP6avVDXrW+N1BT1cYcgzwwipQ3tEL?=
 =?us-ascii?Q?7qvgiWPEF0A/4cTgXcbeLlefLpG+L7a0T0urh6f9zSL7bBh2SoCPioY5V3Kn?=
 =?us-ascii?Q?miWqlk2hJdaT4U6xC/DvY+UUuxAABXuD+sPZPPN2ptIhyQQxa1bCt2PK7l3V?=
 =?us-ascii?Q?wDswpNUhGw8TOgY5/cgPZt2FFKWWEAjNXX65kIh+hJla40lKX26dupyNukvK?=
 =?us-ascii?Q?WGAGei7ZJw0qprVX6qifxcdU2XeNd8rkTUVErRBYz8fk3by6gxtz4hz3k/vU?=
 =?us-ascii?Q?rQMRqxS3oOfn5G2kzTZBa3PTgz5ntRumD4iE/lA4Km1GjSjcmfzrBdzhPwqm?=
 =?us-ascii?Q?Zshyrjmuj66LtRijZAg4CyxePEqivvYZ8+ZpGbx4N5aRgUc3gkpdwS0Oe0wf?=
 =?us-ascii?Q?TMR7LarhF7pj7p4ysgsVif5FuWgYweQV0k9OLds3NCzKrUPM6aDuRRboxiIJ?=
 =?us-ascii?Q?oPF/IiR/WnWbgb3VHwUliw8kOOyvxVgaRzfygUMYd/thmyM/Yh7GI2UoiWb9?=
 =?us-ascii?Q?ezl2GsS9HyZg2ff3lAoEpr3CV7Kg7yT/iYF/e2bX6bnewdVncFId6aQVVgFd?=
 =?us-ascii?Q?mDRARMRlkB0ktqHxAascIab0vmt7iqVkUrmpM1N0iwGku/U/zRjgcjA+Yd9F?=
 =?us-ascii?Q?lCsvYmgY8wB3c4LVgPTap9eQrbMwgRHyGi+s0CdObvOySU7B6UaVJNSSJevX?=
 =?us-ascii?Q?a5O7ppox/C4glP8irQJu8TBUaI+ZIMViw+2DdI3xkzb4nJ/mkrg2uqc+Awqs?=
 =?us-ascii?Q?jrtnJYS+MEweARK976oDEIQig4UtVZDLZms7uS8YVMpjEjRc7kaL9oOdM23+?=
 =?us-ascii?Q?+DEdULAtKXZcV+na2jODDNz/b8lqJ6odbQH8bGBuoijsuJSPjFtDD7QDoVaN?=
 =?us-ascii?Q?zoMmoJ+V2wGW1adQy3PWvZCDArP1x1+1EU5i2KmxqgluEwTQoOqX8YlDwsk/?=
 =?us-ascii?Q?mtFI6EdupblbvfgJZSgwrkqjmtPbLQzTnIFmmwdzx2rFWfkvpf4PDTdAQkU1?=
 =?us-ascii?Q?bg=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: Nky0mArsPMZgkxkXjT7mYMXVWH05cMXY3xUzGtk1f5O3ki5DYvAxayq41AuU8ktT5uVsXioWdtg96NBUz7iX37fDkjO1K4d4lOdX7qWJyLLG9hu8DJqt+8XoKCfCylS2yL+vTxCjK91pxZvVi0JLQsVa6zEiPb99C5pUXl/x6ew+J0nqiav7iI5aDcNJLNdMs5w1PHNhp8EekpDgVTFxz6rhCifXk3hzkYA1vokfhKzAYAd5ELF/JUSg1DkFdfiTzbkBGaosKG1hNNv7NUmQESQi9DQvp4Pjtji7pOf+DdMfA1Q0vRKf8pxI2p/Jjq1B6djV2OOV7ofiGWRcv8xMo2a9QQDYCJ5mbMJX8bwFd4oMFtaowRRDYy2YYPIZAG7XV6OWq1CzK1RTRXNV2U0ajqYcsHkKo50IES7bvBeEghVSBK137R5TcLGW5+yzCYU4h9ZLmCYReTIghPuRzKZKh9YHsSEofgjvx/nYxwbMR7DWpHcb/RakStVm+9fjW2t4ASWuIOiSHcPrTBj9f3BLRxgqEj3ENUoj66+fL/76VBvMXq0WU8nOyh0ProkaJov779kfN8V/pnu4X75LNcjQ0OvukVXpzBRFcAk+mUUYq5NcUNzMdUun5ZuYGjvi7k9A4MgHnV4oep+4qO905k9XuRl4WRLmL/Gb7658fTncghmXz6tAZ+ff2PcQjsMT+4F7Lj0DsXpp36fiReJkerQLaOLVkSXFp4PGzMErwNPHeo+8pg67Ytc3gVSgT/awmymM8rOcmWBVORKUl/aFNIj904CH5yi38o1uwqWGf8rAZHS/iRltoUnlbGt2Yc4/Xx0t
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d56ac490-d908-4ccf-5e3a-08db59997ccf
X-MS-Exchange-CrossTenant-AuthSource: PH0PR10MB4759.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 May 2023 01:19:56.3573
 (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: ESYQD409vO6EdmMX+NjEZ/M3joAZIa8CY55KkjbJPl3xHgKFgdJNLBd4dAEQKzpOl0+BD1IrM+SQEu56p4F68hhoZPMtd+u/SLF8ojgayH4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6966
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-20_16,2023-05-17_02,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 bulkscore=0
 mlxlogscore=776 mlxscore=0 malwarescore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000
 definitions=main-2305210007
X-Proofpoint-ORIG-GUID: f6GjF8sGHOL7uyWC4DNCXSRLRkztBjeR
X-Proofpoint-GUID: f6GjF8sGHOL7uyWC4DNCXSRLRkztBjeR
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,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


John,

> @Martin, Do you have any preference for what we do now? This code
> which does not check for error and does not pre-zero sshdr is
> longstanding, so I am not sure if Juergen's change is required for for
> v6.4. I'm thinking to fix callers for v6.5 and also maybe change the
> API, as I described.

As I alluded to in the tracing thread, I'd like to see SK/ASC/ASCQ being
generally available in the scsi_cmnd results instead of all this sense
buffer and sense header micromanagement in every caller. That's a pretty
heavy lift, though.

Short term we need all callers to be fixed up. I'm not a particularly
big fan of scsi_execute_cmd() zeroing something being passed in. I
wonder if it would be worth having a DECLARE_SENSE_HEADER()?

-- 
Martin K. Petersen	Oracle Linux Engineering