linux-kernel - Re: [PATCH] scsi: Let scsi_execute_cmd() mark args->sshdr as invalid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 18 May 2023 06:53:14 +0200
From:   Juergen Gross <jgross@...e.com>
To:     John Garry <john.g.garry@...cle.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>
Cc:     linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>, stable@...r.kernel.org
Subject: Re: [PATCH] scsi: Let scsi_execute_cmd() mark args->sshdr as invalid

On 17.05.23 17:05, John Garry wrote:
> 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)?

I don't know.

I've stumbled over the problem while looking into the code due to analyzing a
customer's problem. I'm no SCSI expert, but the customer was running Xen and
there was the suspicion this could be an underlying Xen issue (which is my
area of interest).

It became clear rather quickly that the uninitialized sshdr wasn't the root
cause of the customer's problems, but I thought it should be fixed anyway. As
there seem to be quite some problematic callers of scsi_execute_cmd(), I've
chosen to add the minimal needed initialization of sshdr to scsi_execute_cmd()
instead of trying to fix all callers.

Reasoning why the code is looking like it does is surely not what _I_ want to
do.

Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)