linux-kernel - Re: [PATCH 3.6 1/2] target: remove pscsi_clear_cdb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 06 Sep 2012 20:22:53 -0700
From:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	linux-kernel@...r.kernel.org, target-devel@...r.kernel.org
Subject: Re: [PATCH 3.6 1/2] target: remove pscsi_clear_cdb_lun

On Thu, 2012-09-06 at 22:51 +0200, Paolo Bonzini wrote:
> Il 06/09/2012 20:58, Nicholas A. Bellinger ha scritto:
> > On Thu, 2012-09-06 at 17:13 +0200, Paolo Bonzini wrote:
> >> The purpose of this function is to clear a LUN set in the CDB, in case
> >> the initiator talking to us is speaking an old standards version.
> >> However, as things stand, pscsi_clear_cdb_lun has two problems.  It
> >> will "deceive" the guest by clearing the LUN bits on initial
> >> commands (INQUIRY, TEST UNIT READY, etc.); but then it will let the
> >> LUN bits through on reads, which will likely fail due to protection not
> >> being enabled.  Second, not all commands are properly filtered, in
> >> particular WRITE's WRPROTECT bits are cleared.
> >>
> >> This should be done by the fabric module rather than by PSCSI, if it
> >> knows such initiators may be lying around _and_ it can assume that its
> >> initiators do not really care about protection information.  Nuke it
> >> from PSCSI.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> >> ---
> > 
> > NAK.  This code was originally added to prevent certain pSCSI HBAs from
> > going bonkers when they got a legacy SCSI LUN ID encoded within the CDB
> > (eg: the fabric LUN ID) that's different from the physical SCSI LUN ID
> > on an Parallel SCSI bus.
> 
> I understood, but what's good in making INQUIRY work, if the HBA will
> equally go bonkers on the first actual I/O (READ/WRITE/VERIFY are all
> affected)?
> 

The original purpose of pscsi_clear_cdb_lun() is to clear byte 1 bit 0-5
for all incoming CDBs in SCSI-2 that did encode LUN ID into the CDB as a
work-around for (some) Parallel SCSI hardware that depends on this value
to function.

As modern SCSI-3 + SAS fabrics, et al have moved away from doing this
completely, thinking about this some more I would be OK with disabling
this by default for modern SCSI HW if the pSCSI device is reporting >=
SCSI-3 from se_subsystem_api->get_device_rev().

> This is something the LLDs or the fabric module should be doing.  But I
> guess it could be an attribute too.
> 

I think it's safe to disable this for modern SCSI hw, but lets keep it
around for pSCSI backend devices that are reporting SCSI-2 support and
below.

Care to respin..?  ;)

Thanks Paolo!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/