|Summary:||[PATCH] ATA IDENTIFY DEVICE sent with SECTOR_COUNT=0|
|Product:||udisks||Reporter:||Peter Paluch <peter.paluch>|
|Component:||detection||Assignee:||David Zeuthen (not reading bugmail) <zeuthen>|
|Status:||RESOLVED FIXED||QA Contact:|
|i915 platform:||i915 features:|
|Attachments:||Patch to correct IDENTIFY (PACKET) DEVICE calls so that the SECTOR_COUNT is set to 1|
Description Peter Paluch 2014-01-19 01:08:47 UTC
Created attachment 92366 [details] Patch to correct IDENTIFY (PACKET) DEVICE calls so that the SECTOR_COUNT is set to 1 Greetings, The udiskd daemon currently sends ATA IDENTIFY DEVICE pass through command with the SECTOR_COUNT field set to 0. As a result, the ioctl() call returns an empty data buffer. In addition, this has caused my USB drive to stall for 30 seconds after being connected, prompting the kernel to reset it. I have originally contacted the linux-usb mailing list with this issue; the thread can be found here: http://www.spinics.net/lists/linux-usb/msg99732.html I have tried to correct the udisk sources in all instances where I found the ATA IDENTIFY DEVICE and ATA IDENTIFY PACKET DEVICE command calls by making sure that the input.count that translates into cdb and cdb is set to 1. The patch is included. I would like to ask you to kindly review this correction, and if considered appropriate, to incorporate it into udiskd sources. Thank you! Best regards, Peter
Comment 1 David Zeuthen (not reading bugmail) 2014-01-20 17:07:56 UTC
Interesting, nice detective work. I'm not opposed to applying this fix (just tested it, works fine with a couple of ATA devices here) if it fixes real problems, but I don't see anywhere that says that Count should be 1... the ATA8 spec says "N/A". (Btw, there's a ton of other places doing this where you may need the same fix including ata_id, libatasmart, smartmontools. I haven't checked but I think at least ata_id also sets Count to 0.)
Comment 2 Peter Paluch 2014-01-20 18:32:41 UTC
Hi David, (In reply to comment #1) > Interesting, nice detective work. I'm not opposed to applying this fix (just > tested it, works fine with a couple of ATA devices here) if it fixes real > problems, but I don't see anywhere that says that Count should be 1... the > ATA8 spec says "N/A". Thank you for the compliment. It definitely solves a rather annoying problem for me, and obviously for another person as well (Andrej P. who joined my bug report to the linux-usb). I know that ATA8 spec says N/A with regard to the Count field. What the ATA8 also says, however, is: "The IDENTIFY DEVICE command enables the host to receive a 512-byte block of data from the device." It may be related to the SATL layer in particular, because in SCSI / ATA Translation - 3 (SAT-3) Section 188.8.131.52 the spec states: "The SATL shall determine the transfer length by the method specified in the T_LENGTH field as shown in table 153." Now obviously, the transfer length is 512 bytes - this is the result from the IDENTIFY DEVICE. It appears that it is the SATL, not the drive itself, that needs the count=1, otherwise, the SATL does not read out the data from the drive after the command has been sent - that's my guess. > (Btw, there's a ton of other places doing this where you may need the same > fix including ata_id, libatasmart, smartmontools. I haven't checked but I > think at least ata_id also sets Count to 0.) I believe the opposite is true. I've just checked the source files: - sg_sat_identify from sg3-utils sets count=1 - smartmontools set count=1 - ata_id sets count=1 - libatasmart sets count=1 I can point you towards the particular source files and lines. You may want to check it yourself even without source files - simply run the binaries in strace and check for the ioctl (fd, SG_IO) calls. The hexa dump of the cdb is fairly easily identifiable. Best regards, Peter
Comment 3 David Zeuthen (not reading bugmail) 2014-01-20 20:18:59 UTC
I just checked udev's ata_id (I wrote parts of it), and, indeed you are correct. I've applied your patch, see http://cgit.freedesktop.org/udisks/commit/?id=81a350fca9431048175fcf639909d988aceb2a17 Thanks!
Comment 4 Peter Paluch 2014-01-20 23:06:39 UTC
David, Thank you! It was an honor to help. Best regards, Peter (In reply to comment #3) > I just checked udev's ata_id (I wrote parts of it), and, indeed you are > correct. I've applied your patch, see > > http://cgit.freedesktop.org/udisks/commit/ > ?id=81a350fca9431048175fcf639909d988aceb2a17 > > Thanks!
Comment 5 Andrej Polak 2014-01-21 13:26:21 UTC
Patch from Peter works. Good job.