Bug 73780 - [PATCH] ATA IDENTIFY DEVICE sent with SECTOR_COUNT=0
Summary: [PATCH] ATA IDENTIFY DEVICE sent with SECTOR_COUNT=0
Status: RESOLVED FIXED
Alias: None
Product: udisks
Classification: Unclassified
Component: detection (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: David Zeuthen (not reading bugmail)
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-19 01:08 UTC by Peter Paluch
Modified: 2014-01-21 13:26 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Patch to correct IDENTIFY (PACKET) DEVICE calls so that the SECTOR_COUNT is set to 1 (1.48 KB, text/plain)
2014-01-19 01:08 UTC, Peter Paluch
Details

Note You need to log in before you can comment on or make changes to this bug.
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[5] and cdb[6] 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 12.2.2.4 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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.