linux-kernel - Re: [PATCH 3.6 2/2] target: use a bounce buffer in transport_kmap_data

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]

Message-ID: <5049E208.3040109@redhat.com>
Date:	Fri, 07 Sep 2012 14:01:12 +0200
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
CC:	Christoph Hellwig <hch@....de>, Roland Dreier <roland@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	target-devel <target-devel@...r.kernel.org>
Subject: Re: [PATCH 3.6 2/2] target: use a bounce buffer in transport_kmap_data_sg
 for 0 or 1-page sglist

Il 07/09/2012 05:35, Nicholas A. Bellinger ha scritto:
>> It is not completely correct for virtual backends, for example I think
>> it will always return success for 0-block reads or writes, even if the
>> start LBA is out of range.  This is also something that I saw with
>> PSCSI, and is fixed by these patches.
> 
> Good point !

I spent the morning digging further down the rabbithole, and I found the
following testcases:

REPORT LUNS:
sg_raw -r8 /dev/sdb a0 00 00 00 00 00 00 00 00 08 00 00
    should fail with ILLEGAL REQUEST / INVALID FIELD IN CDB sense
    does not fail

INQUIRY (VPD PAGE != 0, std != 0):
sg_raw /dev/sdb 12 00 83 00 00 00
    should fail with ILLEGAL REQUEST / INVALID FIELD IN CDB sense
    does not fail

MODE SENSE off by one (by two for 10-byte CDB):
sg_raw -r20 /dev/sdb 5a 00 0a 00 00 00 00 00 14 00
    last byte should be 0x1e
    it is 0x00

READ:
Testcase: sg_raw /dev/sdb 28 00 80 00 00 00 00 00 00 00
    should fail with ILLEGAL REQUEST / LBA OUT OF RANGE sense
    does not fail

Plus:

- missing checks on parameter list length for PR OUT with SPEC_I_PT,
UNMAP, SET TARGET PORT GROUPS.  In some cases these could lead to
reading undefined data.

- no checks for OOM in callers of transport_kmem_data_sg.

>> Also, even though it handles zero-size, it doesn't handle a CDB with a
>> small but nonzero allocation length.  If you have such a CDB, you can
>> overflow the sglist. 
> 
> Any particular sg_raw example in mind that can trigger this..?

sg_raw (or even SG_IO) doesn't work because misaligned scatterlists are
bounce-buffered by the block layer in blk_map_rq_user.

However, given the above bugs it's better to attack the callers of
transport_kmem_data_sg one by one.  Again, zero-length CDB support comes
for free.

> Ok, if this is a genuine issue then please show how to trigger with
> sg_raw, and let's plan on merging this as a -rc6 bugfix in order to
> spend some more testing w/ scsi-testsuite across different backends over
> the next week.

Doesn't seem to be too important, it can be done for 3.7 except perhaps
for PSCSI; I'll put the PSCSI patch at the beginning of the series.  I
can make scsi-testsuite patches for the above issues, but I'll be glad
if someone beats me to it.

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/