Re: [PATCH v2] scsi: stex: properly zero out the passthrough command structure

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Sep 26, 2022 at 08:54:24AM -0700, Lee Duncan wrote:
> On 9/9/22 09:24, Bart Van Assche wrote:
> > On 9/8/22 23:54, Greg Kroah-Hartman wrote:
> > > From: Linus Torvalds <torvalds@linux-foundation.org>
> > > 
> > > The passthrough structure is declared off of the stack, so it needs to
> > > be set to zero before copied back to userspace to prevent any
> > > unintentional data leakage.  Switch things to be statically allocated
> > > which will fill the unused fields with 0 automatically.
> > > 
> > > Reported-by: hdthky <hdthky0@gmail.com>
> > > Cc: stable <stable@kernel.org>
> > > Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> > > Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> > > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > ---
> > >   v2: Linus's updated version that moves the initialization to be
> > >       statically defined and changes the function prototype and structure
> > >       to be const.
> > > 
> > >   drivers/scsi/stex.c      | 17 +++++++++--------
> > >   include/scsi/scsi_cmnd.h |  2 +-
> > >   2 files changed, 10 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
> > > index e6420f2127ce..8def242675ef 100644
> > > --- a/drivers/scsi/stex.c
> > > +++ b/drivers/scsi/stex.c
> > > @@ -665,16 +665,17 @@ static int stex_queuecommand_lck(struct
> > > scsi_cmnd *cmd)
> > >           return 0;
> > >       case PASSTHRU_CMD:
> > >           if (cmd->cmnd[1] == PASSTHRU_GET_DRVVER) {
> > > -            struct st_drvver ver;
> > > +            const struct st_drvver ver = {
> > > +                .major = ST_VER_MAJOR,
> > > +                .minor = ST_VER_MINOR,
> > > +                .oem = ST_OEM,
> > > +                .build = ST_BUILD_VER,
> > > +                .signature[0] = PASSTHRU_SIGNATURE,
> > > +                .console_id = host->max_id - 1,
> > > +                .host_no = hba->host->host_no,
> > > +            };
> > >               size_t cp_len = sizeof(ver);
> > > -            ver.major = ST_VER_MAJOR;
> > > -            ver.minor = ST_VER_MINOR;
> > > -            ver.oem = ST_OEM;
> > > -            ver.build = ST_BUILD_VER;
> > > -            ver.signature[0] = PASSTHRU_SIGNATURE;
> > > -            ver.console_id = host->max_id - 1;
> > > -            ver.host_no = hba->host->host_no;
> > >               cp_len = scsi_sg_copy_from_buffer(cmd, &ver, cp_len);
> > >               if (sizeof(ver) == cp_len)
> > >                   cmd->result = DID_OK << 16;
> > > diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> > > index bac55decf900..7d3622db38ed 100644
> > > --- a/include/scsi/scsi_cmnd.h
> > > +++ b/include/scsi/scsi_cmnd.h
> > > @@ -201,7 +201,7 @@ static inline unsigned int scsi_get_resid(struct
> > > scsi_cmnd *cmd)
> > >       for_each_sg(scsi_sglist(cmd), sg, nseg, __i)
> > >   static inline int scsi_sg_copy_from_buffer(struct scsi_cmnd *cmd,
> > > -                       void *buf, int buflen)
> > > +                       const void *buf, int buflen)
> > >   {
> > >       return sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
> > >                      buf, buflen);
> > 
> > Please split this patch into one patch for the SCSI core and another patch
> > for the STEX driver.
> > 
> > Thanks,
> > 
> > Bart.
> 
> Ping? Is this patch going to stand as is, or are we going to get a V3 that
> addresses Bart's request?

I'll try to do a v3 when I get a chance later this week.

thanks,

greg k-h