| Message ID | YxrjN3OOw2HHl9tx@kroah.com (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [v2] 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.
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
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);