[PATCH] scsi: Fixup fixed sense generation

[PATCH] scsi: Fixup fixed sense generation

[Hannes Reinecke]

Fixed sense reserves only 32 bits for the 'information' field,
so we need to restrict the 'information' value to avoid sense
data corruption.
Also the sense key is only 4 bits.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/scsi_error.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 106884a..ba7ffc4 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2510,13 +2510,13 @@ void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq)
 {
 	if (desc) {
 		buf[0] = 0x72;	/* descriptor, current */
-		buf[1] = key;
+		buf[1] = key & 0x0f;
 		buf[2] = asc;
 		buf[3] = ascq;
 		buf[7] = 0;
 	} else {
 		buf[0] = 0x70;	/* fixed, current */
-		buf[2] = key;
+		buf[2] = key & 0x0f;
 		buf[7] = 0xa;
 		buf[12] = asc;
 		buf[13] = ascq;
@@ -2549,7 +2549,11 @@ void scsi_set_sense_information(u8 *buf, u64 info)
 		put_unaligned_be64(info, &ucp[4]);
 	} else if ((buf[0] & 0x7f) == 0x70) {
 		buf[0] |= 0x80;
-		put_unaligned_be64(info, &buf[3]);
+		/*
+		 * Fixed format sense reserves only 32 bits for the
+		 * 'information' field
+		 */
+		put_unaligned_be32((u32)info, &buf[3]);
 	}
 }
 EXPORT_SYMBOL(scsi_set_sense_information);
-- 
1.8.5.2

Re: [PATCH] scsi: Fixup fixed sense generation

[Bart Van Assche]

On 07/31/15 06:05, Hannes Reinecke wrote:
> Fixed sense reserves only 32 bits for the 'information' field,
> so we need to restrict the 'information' value to avoid sense
> data corruption.
> Also the sense key is only 4 bits.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>   drivers/scsi/scsi_error.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 106884a..ba7ffc4 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -2510,13 +2510,13 @@ void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq)
>   {
>   	if (desc) {
>   		buf[0] = 0x72;	/* descriptor, current */
> -		buf[1] = key;
> +		buf[1] = key & 0x0f;
>   		buf[2] = asc;
>   		buf[3] = ascq;
>   		buf[7] = 0;
>   	} else {
>   		buf[0] = 0x70;	/* fixed, current */
> -		buf[2] = key;
> +		buf[2] = key & 0x0f;
>   		buf[7] = 0xa;
>   		buf[12] = asc;
>   		buf[13] = ascq;

Please split these changes into two separate patches - one patch for the 
scsi_build_sense_buffer() changes and another patch for the 
scsi_set_sense_information() changes. Please also add a statement that 
triggers a kernel warning if an out-of-range key value is passed to 
scsi_build_sense_buffer() instead of ignoring out-of-range sense key 
values silently.

Thanks,

Bart.

Re: [PATCH] scsi: Fixup fixed sense generation

[Sagi Grimberg]

> -		put_unaligned_be64(info, &buf[3]);
> +		/*
> +		 * Fixed format sense reserves only 32 bits for the
> +		 * 'information' field
> +		 */
> +		put_unaligned_be32((u32)info, &buf[3]);
>   	}
>   }
>   EXPORT_SYMBOL(scsi_set_sense_information);
>

Hannes,

This bit is handled in "scsi: Fix sense information setting in fixed 
sized format"
see http://permalink.gmane.org/gmane.linux.scsi/103115

Can you make it incremental to this patch?

James, I don't see it on the scsi tree, can you pick it up?

Sagi.

Re: [PATCH] scsi: Fixup fixed sense generation

[Hannes Reinecke]

On 08/11/2015 08:49 AM, Sagi Grimberg wrote:
> 
>> -        put_unaligned_be64(info, &buf[3]);
>> +        /*
>> +         * Fixed format sense reserves only 32 bits for the
>> +         * 'information' field
>> +         */
>> +        put_unaligned_be32((u32)info, &buf[3]);
>>       }
>>   }
>>   EXPORT_SYMBOL(scsi_set_sense_information);
>>
> 
> Hannes,
> 
> This bit is handled in "scsi: Fix sense information setting in fixed
> sized format"
> see http://permalink.gmane.org/gmane.linux.scsi/103115
> 
> Can you make it incremental to this patch?
> 
Sure, I will.

> James, I don't see it on the scsi tree, can you pick it up?
> 
James, please. The patch already has the required Reviewed-by tags.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html