From mboxrd@z Thu Jan 1 00:00:00 1970 From: Martin Svec Subject: Re: [PATCH 4/5] target: Skip non hex characters for EVPD=0x83 NAA IEEE Registered Extended Date: Fri, 16 Sep 2011 15:59:09 +0200 Message-ID: <4E73562D.9060205@zoner.cz> References: <1316169506-4441-1-git-send-email-nab@linux-iscsi.org> <1316169506-4441-5-git-send-email-nab@linux-iscsi.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-2; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from ham1.zoner.com ([217.198.112.147]:55885 "EHLO ham1.zoner.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753431Ab1IPN72 (ORCPT ); Fri, 16 Sep 2011 09:59:28 -0400 In-Reply-To: <1316169506-4441-5-git-send-email-nab@linux-iscsi.org> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: "Nicholas A. Bellinger" Cc: target-devel , linux-scsi , Linus Torvalds , Douglas Gilbert , James Bottomley 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 > > 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 > Cc: Martin Svec > Cc: Douglas Gilbert > Cc: James Bottomley > Signed-off-by: Nicholas Bellinger > --- > 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 > +#include > #include > #include > > @@ -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);