* Re: [PATCH] scsi_common: do not clobber fixed sense information
2016-03-18 8:01 [PATCH] scsi_common: do not clobber fixed sense information Hannes Reinecke
@ 2016-03-18 12:57 ` Ewan D. Milne
2016-03-18 16:02 ` Lee Duncan
2016-03-18 16:19 ` Bart Van Assche
2 siblings, 0 replies; 4+ messages in thread
From: Ewan D. Milne @ 2016-03-18 12:57 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, Bart van Assche,
James Bottomley, linux-scsi, Hannes Reinecke
On Fri, 2016-03-18 at 09:01 +0100, Hannes Reinecke wrote:
> For fixed sense the information field is 32 bits, to we need
> to truncate the information field to avoid clobbering the
> sense code.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
> drivers/scsi/scsi_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
> index c126966..3459009 100644
> --- a/drivers/scsi/scsi_common.c
> +++ b/drivers/scsi/scsi_common.c
> @@ -279,7 +279,7 @@ int scsi_set_sense_information(u8 *buf, int buf_len, u64 info)
> put_unaligned_be64(info, &ucp[4]);
> } else if ((buf[0] & 0x7f) == 0x70) {
> buf[0] |= 0x80;
> - put_unaligned_be64(info, &buf[3]);
> + put_unaligned_be32((u32)info, &buf[3]);
> }
>
> return 0;
Well, not clobbering the ADDITIONAL SENSE LENGTH field is good,
however according to SPC-5 what we are really supposed to do here
is not set the VALID bit (buf[0] |= 0x80) if the value to be
returned can't be represented in 32 bits (and set the INFORMATION
field to a "vendor specific" value, whatever that means).
I'm not a T10 member so I don't have immediate access to what the
earlier SPC revisions say.
Reviewed-by: Ewan D. Milne <emilne@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] scsi_common: do not clobber fixed sense information
2016-03-18 8:01 [PATCH] scsi_common: do not clobber fixed sense information Hannes Reinecke
2016-03-18 12:57 ` Ewan D. Milne
@ 2016-03-18 16:02 ` Lee Duncan
2016-03-18 16:19 ` Bart Van Assche
2 siblings, 0 replies; 4+ messages in thread
From: Lee Duncan @ 2016-03-18 16:02 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, Bart van Assche, James Bottomley, linux-scsi,
Hannes Reinecke
On 03/18/2016 01:01 AM, Hannes Reinecke wrote:
> For fixed sense the information field is 32 bits, to we need
> to truncate the information field to avoid clobbering the
> sense code.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
> drivers/scsi/scsi_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
> index c126966..3459009 100644
> --- a/drivers/scsi/scsi_common.c
> +++ b/drivers/scsi/scsi_common.c
> @@ -279,7 +279,7 @@ int scsi_set_sense_information(u8 *buf, int buf_len, u64 info)
> put_unaligned_be64(info, &ucp[4]);
> } else if ((buf[0] & 0x7f) == 0x70) {
> buf[0] |= 0x80;
> - put_unaligned_be64(info, &buf[3]);
> + put_unaligned_be32((u32)info, &buf[3]);
> }
>
> return 0;
>
Reviewed-by: Lee Duncan <lduncan@suse.com>
--
Lee
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] scsi_common: do not clobber fixed sense information
2016-03-18 8:01 [PATCH] scsi_common: do not clobber fixed sense information Hannes Reinecke
2016-03-18 12:57 ` Ewan D. Milne
2016-03-18 16:02 ` Lee Duncan
@ 2016-03-18 16:19 ` Bart Van Assche
2 siblings, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2016-03-18 16:19 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke
On 03/18/2016 01:01 AM, Hannes Reinecke wrote:
> For fixed sense the information field is 32 bits, to we need
> to truncate the information field to avoid clobbering the
> sense code.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
> drivers/scsi/scsi_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
> index c126966..3459009 100644
> --- a/drivers/scsi/scsi_common.c
> +++ b/drivers/scsi/scsi_common.c
> @@ -279,7 +279,7 @@ int scsi_set_sense_information(u8 *buf, int buf_len, u64 info)
> put_unaligned_be64(info, &ucp[4]);
> } else if ((buf[0] & 0x7f) == 0x70) {
> buf[0] |= 0x80;
> - put_unaligned_be64(info, &buf[3]);
> + put_unaligned_be32((u32)info, &buf[3]);
> }
>
> return 0;
Hello Hannes,
Please Cc stable for this patch.
Are you aware that the explicit cast to u32 can be left out? Without
that cast the compiler implicitly performs casting from u64 to u32.
Anyway:
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
^ permalink raw reply [flat|nested] 4+ messages in thread