Re: [PATCH 4/5] target: Skip non hex characters for EVPD=0x83 NAA IEEE Registered Extended

[Douglas Gilbert <dgilbert@interlog.com>]

A documentation note: it should be "VPD=0x83" not "EVPD=0x83"
in the subject line and elsewhere.

The "E" stands for "enable" and EVPD refers to a bit field in a
INQUIRY cdb. When it is set the response should be a VPD page
corresponding to the "page code" field in the cdb.

I made the same mistake in my tools but I hope that has all
been cleaned up now.

Doug Gilbert


On 11-09-16 09:59 AM, Martin Svec wrote:
> Tested, looks fine to me. (For next == 0, maybe use assignment instead of
> bitwise OR in "buf[cnt] |= hex_to_bin(*p++) << 4;"?)
>
> Martin
>
> Dne 16.9.2011 12:38, Nicholas A. Bellinger napsal(a):
>> From: Nicholas Bellinger<nab@linux-iscsi.org>
>>
>> This patch adds target_parse_naa_6h_vendor_specific() to address a bug where the
>> conversion of PRODUCT SERIAL NUMBER to use hex2bin() in target_emulate_evpd_83()
>> was not doing proper isxdigit() checking. This conversion of the vpd_unit_serial
>> configifs attribute is done while generating a EVPD=0x83 NAA IEEE Registered
>> Extended DESIGNATOR format's 100 bits of unique VENDOR SPECIFIC IDENTIFIER +
>> VENDOR SPECIFIC IDENTIFIER EXTENSION area.
>>
>> This patch allows vpd_unit_serial (EVPD=0x80) and the T10 Vendor ID DESIGNATOR
>> format (EVPD=0x83) to continue to use free-form variable length ASCII values,
>> and now skips any non hex characters for fixed length NAA IEEE Registered
>> Extended
>> DESIGNATOR format (EVPD=0x83) requring the binary conversion.
>>
>> This was originally reported by Martin after the v3.1-rc1 change to use hex2bin()
>> in commit 11650b859681e03fdbf26277fcfc5f1f62186703 where the use of non hex
>> characters in vpd_unit_serial generated different values than the original
>> v3.0 internal hex -> binary code. This v3.1 change caused a problem with
>> filesystems who write a NAA DESIGNATOR onto it's ondisk metadata, and this patch
>> will (again) change existing values to ensure that non hex characters are not
>> included in the fixed length NAA DESIGNATOR.
>>
>> Note this patch still expects vpd_unit_serial to be set via existing userspace
>> methods of uuid generation, and does not do strict formatting via configfs input.
>>
>> The original bug report and thread can be found here:
>>
>> NAA breakage
>> http://www.spinics.net/lists/target-devel/msg00477.html
>>
>> The v3.1-rc1 formatting of EVPD=0x83 w/o this patch:
>>
>> VPD INQUIRY: Device Identification page
>> Designation descriptor number 1, descriptor length: 20
>> designator_type: NAA, code_set: Binary
>> associated with the addressed logical unit
>> NAA 6, IEEE Company_id: 0x1405
>> Vendor Specific Identifier: 0xffde35ebf
>> Vendor Specific Identifier Extension: 0x3092f498ffa820f9
>> [0x6001405ffde35ebf3092f498ffa820f9]
>> Designation descriptor number 2, descriptor length: 56
>> designator_type: T10 vendor identification, code_set: ASCII
>> associated with the addressed logical unit
>> vendor id: LIO-ORG
>> vendor specific: IBLOCK:ffde35ec-3092-4980-a820-917636ca54f1
>>
>> The v3.1-final formatting of EVPD=0x83 w/ this patch:
>>
>> VPD INQUIRY: Device Identification page
>> Designation descriptor number 1, descriptor length: 20
>> designator_type: NAA, code_set: Binary
>> associated with the addressed logical unit
>> NAA 6, IEEE Company_id: 0x1405
>> Vendor Specific Identifier: 0xffde35ec3
>> Vendor Specific Identifier Extension: 0x924980a82091763
>> [0x6001405ffde35ec30924980a82091763]
>> Designation descriptor number 2, descriptor length: 56
>> designator_type: T10 vendor identification, code_set: ASCII
>> associated with the addressed logical unit
>> vendor id: LIO-ORG
>> vendor specific: IBLOCK:ffde35ec-3092-4980-a820-917636ca54f1
>>
>> Reported-by: Martin Svec<martin.svec@zoner.cz>
>> Cc: Martin Svec<martin.svec@zoner.cz>
>> Cc: Douglas Gilbert<dgilbert@interlog.com>
>> Cc: James Bottomley<jbottomley@parallels.com>
>> Signed-off-by: Nicholas Bellinger<nab@linux-iscsi.org>
>> ---
>> drivers/target/target_core_cdb.c | 35 +++++++++++++++++++++++++++++++++--
>> 1 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/target/target_core_cdb.c b/drivers/target/target_core_cdb.c
>> index 89ae923..08bda0c 100644
>> --- a/drivers/target/target_core_cdb.c
>> +++ b/drivers/target/target_core_cdb.c
>> @@ -24,6 +24,7 @@
>> */
>>
>> #include<linux/kernel.h>
>> +#include<linux/ctype.h>
>> #include<asm/unaligned.h>
>> #include<scsi/scsi.h>
>>
>> @@ -154,6 +155,37 @@ target_emulate_evpd_80(struct se_cmd *cmd, unsigned char
>> *buf)
>> return 0;
>> }
>>
>> +static void
>> +target_parse_naa_6h_vendor_specific(struct se_device *dev, unsigned char
>> *buf_off)
>> +{
>> + unsigned char *p =&dev->se_sub_dev->t10_wwn.unit_serial[0];
>> + unsigned char *buf = buf_off;
>> + int cnt = 0, next = 1;
>> + /*
>> + * Generate up to 36 bits of VENDOR SPECIFIC IDENTIFIER starting on
>> + * byte 3 bit 4 for NAA IEEE Registered Extended DESIGNATOR field
>> + * format, followed by 64 bits of VENDOR SPECIFIC IDENTIFIER EXTENSION
>> + * to complete the payload. These are based from VPD=0x80 PRODUCT SERIAL
>> + * NUMBER set via vpd_unit_serial in target_core_configfs.c to to ensure
>> + * per device uniqeness.
>> + */
>> + while (p != NULL) {
>> + if (cnt>= 13)
>> + break;
>> + if (!isxdigit(*p)) {
>> + p++;
>> + continue;
>> + }
>> + if (next != 0) {
>> + buf[cnt++] |= hex_to_bin(*p++);
>> + next = 0;
>> + } else {
>> + buf[cnt] |= hex_to_bin(*p++)<< 4;
>> + next = 1;
>> + }
>> + }
>> +}
>> +
>> /*
>> * Device identification VPD, for a complete list of
>> * DESIGNATOR TYPEs see spc4r17 Table 459.
>> @@ -219,8 +251,7 @@ target_emulate_evpd_83(struct se_cmd *cmd, unsigned char
>> *buf)
>> * VENDOR_SPECIFIC_IDENTIFIER and
>> * VENDOR_SPECIFIC_IDENTIFIER_EXTENTION
>> */
>> - buf[off++] |= hex_to_bin(dev->se_sub_dev->t10_wwn.unit_serial[0]);
>> - hex2bin(&buf[off],&dev->se_sub_dev->t10_wwn.unit_serial[1], 12);
>> + target_parse_naa_6h_vendor_specific(dev,&buf[off]);
>>
>> len = 20;
>> off = (len + 4);
>
>