Ubuntu
systemd package

ata_id command provides wrong information for ATA devices

  1. Bionic (18.04)
  2. Bug #1979342
Bug #1979342 reported by Ioanna Alifieraki
20
This bug affects 4 people
Affects Status Importance Assigned to Milestone
systemd (Ubuntu)
Fix Released
Medium
Ioanna Alifieraki
Bionic
Confirmed
Medium
Ioanna Alifieraki

Bug Description

[DESCRIPTION]

Original revisions of the SAT (SCSI-ATA Translation) specification
required that all sense data be reported in Descriptor Format (72h).
Later revisions specifcally allow and account for sense data being
reported in Fixed Format (70h).
The current code checks for a Descriptor Format sense structure (0x72),
then looks specifically at the first byte of the first descriptor for the
ATA specific code 0x9, cross referencing it with the first byte which is
just a length field 0x0c (as a sanity check).

The code did not account for the Fixed Format case (0x70).
Upstream commit 402fecff19d42("ata_id: Add check for fixed format sense codes (#13654)")
fixed this by checking for the Fixed Format case(0x70) adn then falling back to
using the top-level SCSI Sense data for the Additional Sense code (0x0) and
then the Additional Sense Code Qualifier (0x1d).

[TEST CASE]

To test this you need and ATA device.

Without the patch id_ata command returns error code 2.

With the patch it returns the correct information :

root@machine:~# ./ata_id /dev/sdn -x
ID_ATA=1
ID_TYPE=disk
ID_BUS=ata
ID_MODEL=HGST_HUH728080ALE604
ID_MODEL_ENC=HGST\x20HUH728080ALE604\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20
ID_REVISION=A4GNW7J0
ID_SERIAL=HGST_HUH728080ALE604_ZZZZH3VX
ID_SERIAL_SHORT=ZZZZH3VX
...

(see commit message for more details)

[REGRESSION POTENTIAL]

This commit changes disk_identify_command() function in src/udev/ata_id/ata_id.c
and therefore any regression potential will affect only this executable (and any other
that may use it).

[OTHER]

The fix is already in Focal.
Currently only Bionic is affected.

Upstream fix :
https://github.com/systemd/systemd/commit/402fecff19d42bb06caed1a3a32262c18087b7f1

Tags: patch sts
Ioanna Alifieraki (joalif)
Changed in systemd (Ubuntu):
importance: Undecided → Medium
assignee: nobody → Ioanna Alifieraki (joalif)
Changed in systemd (Ubuntu Bionic):
status: New → Confirmed
tags: added: sts
Revision history for this message
Ioanna Alifieraki (joalif) wrote :

Debdiff for bionic.

Changed in systemd (Ubuntu Bionic):
assignee: nobody → Ioanna Alifieraki (joalif)
importance: Undecided → Medium
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "lp1979342_bionic.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Revision history for this message
Dan Streetman (ddstreet) wrote :

Unfortunately, while the udev info for affected ATA drives might be 'wrong', that udev info is what all existing Bionic users are currently using, and expecting. Backporting this patch would change the udev info for all Bionic users (with affected ATA drives) which very likely would cause regressions or at least unexpected behavior.

I think the existing behavior of udev in Bionic needs to remain unchanged, and this patch should not be backported.

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in systemd (Ubuntu):
status: New → Confirmed
Revision history for this message
Martin Benicek (xmabeni) wrote :

Hi Dan,
with this change we can align output between bionic and focal version (and Suse linux as well). Our customers are getting inconsistent output while using same command from different environment (bionic, focal, SuSe).
So if we apply the fix - output will be consistent between bionic, focal, Suse, but many users will have to fix their regressions.
If we don't apply fix - users continue to use wrong output, but will not need to spend time to fix regression.

I think the reason not to fix, due to users which will be affected and forced to do adaptation of their tests if not correct. We are not dealing with new feature for bionic, but fixing existing feature.

sincerely
Martin

Revision history for this message
Dan Streetman (ddstreet) wrote :

> I think the reason not to fix, due to users which will be affected and forced to do adaptation of their tests if not correct.

Sorry, no. Existing users on bionic expect their drives to retain the same udev information they have been using, and changing that - even if it is to 'correct' values - is not appropriate.

For example, if someone was referencing their disk in /etc/fstab using its /dev/disk/by-id/ path, they expect that not to change. This patch would (for affected drives) change it. That potentially would break existing users' systems, which isn't acceptable for a stable release.

Changed in systemd (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.