linux-kernel - Re: todays git: WARNING: at drivers/ata/libata-sff.c:1017 ata_sff_hsm

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: <1231600972.3642.22.camel@localhost.localdomain>
Date:	Sat, 10 Jan 2009 09:22:51 -0600
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Sergei Shtylyov <sshtylyov@...mvista.com>,
	Christian Borntraeger <borntraeger@...ibm.com>,
	linux-ide@...r.kernel.org, Jeff Garzik <jgarzik@...hat.com>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: todays git: WARNING: at drivers/ata/libata-sff.c:1017
	ata_sff_hsm_move+0x45e/0x750()

On Sat, 2009-01-10 at 15:12 +0000, Alan Cox wrote:
> O> Your problem is that the *device* is wanting to transfer a set of bytes
> > not divisible by four.  If you want to use word based PIO, you'll have
> > to fall back to collecting bytes for the last two.  Alternatively, you
> 
> Nope.. you can't do that with ATA block transfers - it isn't the same as
> SCSI

Well, in that case you'll have to do byte PIO for all ATAPI devices,
because this is an expected device to host transfer length.

> > could just do byte PIO for all reply lengths like this ... they occur
> > all over the SCSI standard, but not usually in critical paths.
> 
> The problem we have is that the sg list the drivers were given had a
> segment which was not divisible in length by four and was *NOT* the last
> segment in the sg list

Yes it was, look at the debugging info:

Christian Borntraeger wrote:

> [    1.499843] scsi 1:0:0:0: CD-ROM            HL-DT-ST DVDRAM
> GSA-U10N  1.05 PQ: 0 ANSI: 5
> [    1.508389] count: 18 sg->length: 96 qc->cursg_ofs: 0 bytes: 18
> [    1.509369] qc->cdb[]:03000000600000000000000000000000
> That one is broken.

That's a single element sg list with a 96 byte buffer.  That's precisely
what SCSI sets up to receive incoming sense data.  There's basically no
setup commands which are multiple element because the sg list is set up
on a kmalloc'd buffer.

The problem is that bytes is 18 and that comes from this:

	ap->ops->sff_tf_read(ap, &qc->result_tf);
	ireason = qc->result_tf.nsect;
	bc_lo = qc->result_tf.lbam;
	bc_hi = qc->result_tf.lbah;
	bytes = (bc_hi << 8) | bc_lo;

So we're asking the device how much data it wants to transfer. It said
18 bytes.  There's nothing padding or aligning in block can do about
this.

> The logic in the ATA PIO code is basically
> 
> 
> 	for each sg entry
> 
> 		compute the number of bytes to transfer staying within
> the page
> 		transfer that many bytes (but may be more)
> 
> 		if and only if the transfer is NOT the last segment but is
> 			more than the bytes requested - WARN
> 
> 
> The block alignment is set to 4 bytes so the block code should be handing
> down stuff which is safe. Some of the sense and other stuff is "adjusted"
> by the libata-scsi conversion code which at this point I suspect is the
> offender.

It is.  The receive buffer is 96 bytes.  It will be page aligned and 96
is divisible by 4.

> This is why libata uses the pad buffers we don't get to be quite so exact
> about transfer sizes as SCSI is.

The block layer does do pad buffers, but in this case there was no need
of one.

James


--
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/