Re: [PATCH v4 3/5] scsi: Move sense handling routines to scsi_common

2015-07-10 Thread Hannes Reinecke
On 07/08/2015 05:23 PM, Sagi Grimberg wrote:
 On 7/8/2015 6:06 PM, Hannes Reinecke wrote:
 
 We're adding extra fields here, so we need to make sure to not
 overflow the buffer. You probably have to pass in the buffersize
 to avoid an overflow ...
 Yeah, I know, it's theoretical at the moment.
 But there's nothing which prevents anyone to add other fields to it,
 so this field might be the one causing the overflow.
 
 Since this patch is simply a movement of functions I don't think I
 should change any functionality here. Would it be acceptable to fix
 this in an incremental patch?
Sure.

So you can add a

Reviewed-by: Hannes Reinecke h...@suse.de

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries  Storage
h...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 3/5] scsi: Move sense handling routines to scsi_common

2015-07-08 Thread Sagi Grimberg

On 7/8/2015 6:06 PM, Hannes Reinecke wrote:


We're adding extra fields here, so we need to make sure to not
overflow the buffer. You probably have to pass in the buffersize
to avoid an overflow ...
Yeah, I know, it's theoretical at the moment.
But there's nothing which prevents anyone to add other fields to it,
so this field might be the one causing the overflow.


Since this patch is simply a movement of functions I don't think I
should change any functionality here. Would it be acceptable to fix
this in an incremental patch?
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 3/5] scsi: Move sense handling routines to scsi_common

2015-07-08 Thread Hannes Reinecke
On 07/08/2015 04:58 PM, Sagi Grimberg wrote:
 Sense data handling is also done in the target stack.
 Hence, move sense handling routines to scsi_common so
 the target will be able to use them as well.
 
 Signed-off-by: Sagi Grimberg sa...@mellanox.com
 Reviewed-by: Bart Van Assche bart.vanass...@sandisk.com
 Reviewed-by: Christoph Hellwig h...@lst.de
 ---
  drivers/scsi/scsi_common.c | 98 +
  drivers/scsi/scsi_error.c  | 99 
 +-
  include/scsi/scsi_common.h |  5 +++
  include/scsi/scsi_eh.h |  7 +---
  4 files changed, 105 insertions(+), 104 deletions(-)
 
 diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
 index 2ff0922..41432c1 100644
 --- a/drivers/scsi/scsi_common.c
 +++ b/drivers/scsi/scsi_common.c
 @@ -5,6 +5,7 @@
  #include linux/bug.h
  #include linux/kernel.h
  #include linux/string.h
 +#include asm/unaligned.h
  #include scsi/scsi_common.h
  
  /* NB: These are exposed through /proc/scsi/scsi and form part of the ABI.
 @@ -176,3 +177,100 @@ bool scsi_normalize_sense(const u8 *sense_buffer, int 
 sb_len,
   return true;
  }
  EXPORT_SYMBOL(scsi_normalize_sense);
 +
 +/**
 + * scsi_sense_desc_find - search for a given descriptor type in  
 descriptor sense data format.
 + * @sense_buffer:byte array of descriptor format sense data
 + * @sb_len:  number of valid bytes in sense_buffer
 + * @desc_type:   value of descriptor type to find
 + *   (e.g. 0 - information)
 + *
 + * Notes:
 + *   only valid when sense data is in descriptor format
 + *
 + * Return value:
 + *   pointer to start of (first) descriptor if found else NULL
 + */
 +const u8 * scsi_sense_desc_find(const u8 * sense_buffer, int sb_len,
 + int desc_type)
 +{
 + int add_sen_len, add_len, desc_len, k;
 + const u8 * descp;
 +
 + if ((sb_len  8) || (0 == (add_sen_len = sense_buffer[7])))
 + return NULL;
 + if ((sense_buffer[0]  0x72) || (sense_buffer[0]  0x73))
 + return NULL;
 + add_sen_len = (add_sen_len  (sb_len - 8)) ?
 + add_sen_len : (sb_len - 8);
 + descp = sense_buffer[8];
 + for (desc_len = 0, k = 0; k  add_sen_len; k += desc_len) {
 + descp += desc_len;
 + add_len = (k  (add_sen_len - 1)) ? descp[1]: -1;
 + desc_len = add_len + 2;
 + if (descp[0] == desc_type)
 + return descp;
 + if (add_len  0) // short descriptor ??
 + break;
 + }
 + return NULL;
 +}
 +EXPORT_SYMBOL(scsi_sense_desc_find);
 +
 +/**
 + * scsi_build_sense_buffer - build sense data in a buffer
 + * @desc:Sense format (non zero == descriptor format,
 + *  0 == fixed format)
 + * @buf: Where to build sense data
 + * @key: Sense key
 + * @asc: Additional sense code
 + * @ascq:Additional sense code qualifier
 + *
 + **/
 +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[2] = asc;
 + buf[3] = ascq;
 + buf[7] = 0;
 + } else {
 + buf[0] = 0x70;  /* fixed, current */
 + buf[2] = key;
 + buf[7] = 0xa;
 + buf[12] = asc;
 + buf[13] = ascq;
 + }
 +}
 +EXPORT_SYMBOL(scsi_build_sense_buffer);
 +
 +/**
 + * scsi_set_sense_information - set the information field in a
 + *   formatted sense data buffer
 + * @buf: Where to build sense data
 + * @info:64-bit information value to be set
 + *
 + **/
 +void scsi_set_sense_information(u8 *buf, u64 info)
 +{
 + if ((buf[0]  0x7f) == 0x72) {
 + u8 *ucp, len;
 +
 + len = buf[7];
 + ucp = (char *)scsi_sense_desc_find(buf, len + 8, 0);
 + if (!ucp) {
 + buf[7] = len + 0xa;
 + ucp = buf + 8 + len;
 + }
We're adding extra fields here, so we need to make sure to not
overflow the buffer. You probably have to pass in the buffersize
to avoid an overflow ...
Yeah, I know, it's theoretical at the moment.
But there's nothing which prevents anyone to add other fields to it,
so this field might be the one causing the overflow.

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries  Storage
h...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html