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

[Martin Svec <martin.svec@zoner.cz>]

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);