| Message ID | 20220908145154.2284098-1-gregkh@linuxfoundation.org |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: stex: properly zero out the passthrough command structure | expand |
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.
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'd like to know so I can backport the proper patch(es) to address this issue.
diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c index e6420f2127ce..fc5880a35723 100644 --- a/drivers/scsi/stex.c +++ b/drivers/scsi/stex.c @@ -668,6 +668,7 @@ static int stex_queuecommand_lck(struct scsi_cmnd *cmd) struct st_drvver ver; size_t cp_len = sizeof(ver); + memset(&ver, 0x00, sizeof(ver)); ver.major = ST_VER_MAJOR; ver.minor = ST_VER_MINOR; ver.oem = ST_OEM;