Re: [PATCH v2 07/10] qla2xxx: Terminate exchange if corrputed.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/23/16, 1:01 AM, "Bart Van Assche" <Bart.VanAssche@xxxxxxxxxxx> wrote:

>On Wed, 2016-12-21 at 13:57 -0800, Himanshu Madhani wrote:
>> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
>> index f7df01b..b14455e 100644
>> --- a/drivers/scsi/qla2xxx/qla_def.h
>> +++ b/drivers/scsi/qla2xxx/qla_def.h
>> @@ -1556,7 +1556,8 @@ struct link_statistics {
>>  struct atio {
>>  	uint8_t		entry_type;		/* Entry type. */
>>  	uint8_t		entry_count;		/* Entry count. */
>> -	uint8_t		data[58];
>> +	uint16_t	attr_n_length;
>> +	uint8_t		data[56];
>>  	uint32_t	signature;
>>  #define ATIO_PROCESSED 0xDEADDEAD		/* Signature */
>> };
>
>Are struct atio and/or struct atio_from_isp the description of a firmware
>data structure? If so, do all firmware versions initialize the attr_n_length
>field or is there a minimum firmware version required for this field to be
>valid? Please mention any changed dependencies on the firmware version in the
>patch description. Please also fix the typo in the patch title when reposting
>this patch ("corrputed").

They represent same data structure from firmware. We will send follow up patch to
Clean up redundant structure.

>
>> +			/* This packet is corrupted. The header + payload
>> +			 * can not be trusted. There is no point in passing
>> +			 * it further up.
>> +			 */
>
>This is not the proper Linux kernel comment style (see also
>Documentation/process/coding-style.rst).

Ack. Will update patch with correct format.

>
>> diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
>> index f26c5f6..f31d343 100644
>> --- a/drivers/scsi/qla2xxx/qla_target.h
>> +++ b/drivers/scsi/qla2xxx/qla_target.h
>> @@ -427,13 +427,33 @@ struct atio_from_isp {
>>  		struct {
>>  			uint8_t  entry_type;	/* Entry type. */
>>  			uint8_t  entry_count;	/* Entry count. */
>> -			uint8_t  data[58];
>> +			uint16_t attr_n_length;
>> +#define FCP_CMD_LENGTH_MASK 0x0fff
>> +#define FCP_CMD_LENGTH_MIN  0x38
>> +			uint8_t  data[56];
>>  			uint32_t signature;
>>  #define ATIO_PROCESSED 0xDEADDEAD		/* Signature */
>>  		} raw;
>>  	} u;
>>  } __packed;
>>  
>> +static inline int fcpcmd_is_corrupted(struct atio *atio)
>> +{
>> +	if (atio->entry_type == ATIO_TYPE7 &&
>> +	    (le16_to_cpu(atio->attr_n_length & FCP_CMD_LENGTH_MASK) <
>> +	    FCP_CMD_LENGTH_MIN))
>> +		return 1;
>> +	else
>> +		return 0;
>> +}
>
>The above code shows that attr_n_length is a little endian field so please
>declare it as little endian where appropriate (__le16 instead of uint16_t).

Ack. Will update the patch

>
>Bart.
��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux