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

[Juergen Gross]

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

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature