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

["Nicholas A. Bellinger" <nab@linux-iscsi.org>]

On Fri, 2011-09-16 at 15:59 +0200, 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;"?)
> 

Fair enough.  Making this minor change now.

Thanks for testing Martin!

--nab

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