Menu

#87 hdparm --dco-identify reports invalid value for Real max sectors

v1.0 (example)
closed-fixed
nobody
None
5
2021-08-25
2021-05-10
No

hdparm --dco-identify /dev/sdd
/dev/sdd:
DCO Checksum verified.
DCO Revision: 0x0002
The following features can be selectively disabled via DCO:
Transfer modes:
mdma0 mdma1 mdma2
udma0 udma1 udma2 udma3 udma4 udma5 udma6
Real max sectors: 18446744073165333168
ATA command/feature sets:
SMART self_test error_log security 48_bit
SATA command/feature sets:
interface_power_management SSP

expected Real max sectors value is 3750748848.

Raw data read from the device bellow

ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_FROM_DEV, cmd_len=16, cmdp="\x85\x08\x0e\x00\xc2\x00\x01\x00\x00\x00\x00\x00\x00\x40\xb1\x00", mx_sb_len=32, iovec_count=0, dxfer_len=512, timeout=15000, flags=0, dxferp="\x02\x00\x07\x00\x7f\x00\xaf\xe2\x8f\xdf\x00\x00\x00\x00\xaf\x39\x33\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"..., status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, driver_status=0, resid=0, duration=0, info=0}) = 0

reproducer:
cat test.c

#include <stdio.h>

typedef unsigned long long __u64;

int main() {
    unsigned long long lba;
    unsigned char dco2[] ="\x41\x41\x41\x41\x41\x41\xaf\xe2\x8f\xdf\x00\x00\x00\x00\xaf\x39";
    unsigned short *dco = dco2;

    lba = ((((unsigned long long)dco[5]) << 32) | (dco[4] << 16) | dco[3]) + 1;

    printf("\tNo cast: lba=%8llu hex(lba)=%8llx\n", lba, lba);

    lba = ((((unsigned long long)dco[5]) << 32) | ((unsigned long long) dco[4] << 16) | (unsigned long long) dco[3]) + 1;

    printf("\tWith cast: lba=%8llu hex(lba)=%8llx\n", lba, lba);

}

❯ ./test
No cast: lba=18446744073165333168 hex(lba)=ffffffffdf8fe2b0
With cast: lba=3750748848 hex(lba)=df8fe2b0

When we look at https://gcc.godbolt.org/z/dfj4o3xda there is cdqe which does sign extension treating the value as signed thus adding extra 0xffff in the "beginning" if first bit was set to 1. I have no idea why the compiler is treating bit shift output as signed value performing sign extension instead of zero extension. But adding a cast to 64 bit value fixes the issue (no signed extension anymore, please see attached patch).

1 Attachments

Discussion

  • Mark Lord

    Mark Lord - 2021-05-10
    • status: open --> closed-fixed
     
  • Mark Lord

    Mark Lord - 2021-05-10

    Fix this and similar bugs elsewhere in the code. hdparm-9.62 now available.

     
    ❤️
    1
  • Norman Diamond

    Norman Diamond - 2021-08-25

    "I have no idea why the compiler is treating bit shift output as signed value performing sign extension instead of zero extension."

    The reason is that unsigned short promotes to signed int unless short and int have the same length. ISO C language standard, section 6.3.1.1. Mr. Lord fixed hdparm but C is unfixable.

     
    👍
    1

Log in to post a comment.

Monday.com Logo