All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: stex: properly zero out the passthrough command structure
@ 2022-09-08 14:51 Greg Kroah-Hartman
  2022-09-09  6:54 ` [PATCH v2] " Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-08 14:51 UTC (permalink / raw)
  To: jejb, martin.petersen
  Cc: linux-scsi, Greg Kroah-Hartman, hdthky, stable, Dan Carpenter

The passthrough structure is declared off of the stack, so it needs to
be zeroed out before copied back to userspace to prevent any
unintentional data leakage.

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>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/scsi/stex.c | 1 +
 1 file changed, 1 insertion(+)

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;
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2] scsi: stex: properly zero out the passthrough command structure
  2022-09-08 14:51 [PATCH] scsi: stex: properly zero out the passthrough command structure Greg Kroah-Hartman
@ 2022-09-09  6:54 ` Greg Kroah-Hartman
  2022-09-09 16:24   ` Bart Van Assche
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-09  6:54 UTC (permalink / raw)
  To: jejb, martin.petersen, Linus Torvalds
  Cc: linux-scsi, hdthky, stable, Dan Carpenter

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);
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] scsi: stex: properly zero out the passthrough command structure
  2022-09-09  6:54 ` [PATCH v2] " Greg Kroah-Hartman
@ 2022-09-09 16:24   ` Bart Van Assche
  2022-09-26 15:54     ` Lee Duncan
  0 siblings, 1 reply; 5+ messages in thread
From: Bart Van Assche @ 2022-09-09 16:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, jejb, martin.petersen, Linus Torvalds
  Cc: linux-scsi, hdthky, stable, Dan Carpenter

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] scsi: stex: properly zero out the passthrough command structure
  2022-09-09 16:24   ` Bart Van Assche
@ 2022-09-26 15:54     ` Lee Duncan
  2022-09-26 16:17       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: Lee Duncan @ 2022-09-26 15:54 UTC (permalink / raw)
  To: Bart Van Assche, Greg Kroah-Hartman, jejb, martin.petersen,
	Linus Torvalds
  Cc: linux-scsi, hdthky, stable, Dan Carpenter

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.
-- 
Lee Duncan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] scsi: stex: properly zero out the passthrough command structure
  2022-09-26 15:54     ` Lee Duncan
@ 2022-09-26 16:17       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2022-09-26 16:17 UTC (permalink / raw)
  To: Lee Duncan
  Cc: Bart Van Assche, jejb, martin.petersen, Linus Torvalds,
	linux-scsi, hdthky, stable, Dan Carpenter

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-26 17:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-08 14:51 [PATCH] scsi: stex: properly zero out the passthrough command structure Greg Kroah-Hartman
2022-09-09  6:54 ` [PATCH v2] " Greg Kroah-Hartman
2022-09-09 16:24   ` Bart Van Assche
2022-09-26 15:54     ` Lee Duncan
2022-09-26 16:17       ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.