util-linux-2.39.3-1: libblkid returns invalid physical_sector_size

util-linux-2.39.3-1: libblkid returns invalid physical_sector_size

[Christian Franke]

Testcase:

# cygcheck -f /sbin/fdisk.exe
util-linux-2.39.3-1

# /sbin/fdisk.exe -l /dev/sdd
Disk /dev/sdd: 465.76 GiB, 500107862016 bytes, 976773168 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 34359738880 bytes
I/O size (minimum/optimal): 34359738880 bytes / 34359738880 bytes
Disklabel type: dos
Disk identifier: 0x0ac1a23

Device     Boot Start       End   Sectors   Size Id Type
/dev/sdd1        2048 976769023 976766976 465.8G  7 HPFS/NTFS/exFAT

Partition 1 does not start on physical sector boundary.

# printf '0x%016x\n' 34359738880
0x0000000800000200


The problem is that libblkid expects the results of BLKIOMIN, BLKIOOPT 
and BLKPBSZGET as 64 bit 'unsigned long' but Cygwin only returns 32 bit 
'int':

- util-linux-2.39.3/libblkid/src/topology/ioctl.c:
...
static const struct topology_val {

     long  ioc;

     /* functions to set probing result */
     int (*set_ulong)(blkid_probe, unsigned long);
     int (*set_int)(blkid_probe, int);
     int (*set_u64)(blkid_probe, uint64_t);

} topology_vals[] = {
     { BLKALIGNOFF, NULL, blkid_topology_set_alignment_offset },
     { BLKIOMIN, blkid_topology_set_minimum_io_size },
     { BLKIOOPT, blkid_topology_set_optimal_io_size },
     { BLKPBSZGET, blkid_topology_set_physical_sector_size },
#ifdef BLKGETDISKSEQ
     { BLKGETDISKSEQ, .set_u64 = blkid_topology_set_diskseq },
#endif
     /* we read BLKSSZGET in topology.c */
};
...

- util-linux-2.39.3/libblkid/src/topology/topology.c:
...
struct blkid_struct_topology {
     unsigned long    alignment_offset;
     unsigned long    minimum_io_size;
     unsigned long    optimal_io_size;
     unsigned long    logical_sector_size;
     unsigned long    physical_sector_size;
     unsigned long   dax;
     uint64_t    diskseq;
};
...
int blkid_topology_set_physical_sector_size(blkid_probe pr, unsigned 
long val)
...

- newlib-cygwin/winsup/cygwin/fhandler/floppy.cc:
...
     case BLKIOMIN:
       debug_printf ("BLKIOMIN");
       *(int *)buf = (int) bytes_per_sector;
       break;
     case BLKIOOPT:
       debug_printf ("BLKIOOPT");
       *(int *)buf = (int) bytes_per_sector;
       break;
     case BLKPBSZGET:
       debug_printf ("BLKPBSZGET");
       *(int *)buf = (int) bytes_per_sector;
       break;
...


A quick fix which only works on LE platforms:

- util-linux-2.39.3/libblkid/src/topology/ioctl.c:
...
static int probe_ioctl_tp(blkid_probe pr,
...
         union {
             unsigned long ul;
             int i;
             uint64_t u64;
-        } data;
+        } data = { 0 };
...


Downgrading to util-linux-2.33.3-3 does not help. The related code 
differs, but has the same problem.

The fdisk variant in busybox-1.36.1-2 is not affected.

-- 
Regards,
Christian

Re: util-linux-2.39.3-1: libblkid returns invalid physical_sector_size

[Mark Geisert]

Hi Christian,

On 3/31/2024 1:11 AM, Christian Franke via Cygwin wrote:
> Testcase:
> 
> # cygcheck -f /sbin/fdisk.exe
> util-linux-2.39.3-1
> 
> # /sbin/fdisk.exe -l /dev/sdd
> Disk /dev/sdd: 465.76 GiB, 500107862016 bytes, 976773168 sectors
> Units: sectors of 1 * 512 = 512 bytes
> Sector size (logical/physical): 512 bytes / 34359738880 bytes
> I/O size (minimum/optimal): 34359738880 bytes / 34359738880 bytes
[...valuable investigation and patch suggestion elided...]

Your suggested patch looks fine to me.  I have added it to the patch 
deck for a new util-linux 2.39.3-2, which has just been uploaded.  The 
patch allows fdisk.exe to report the three correct values in my limited 
testing.
Thanks for the report and the patch!

..mark

Re: util-linux-2.39.3-1: libblkid returns invalid physical_sector_size

[Christian Franke]

Hi Mark,

Mark Geisert via Cygwin wrote:
> Hi Christian,
>
> On 3/31/2024 1:11 AM, Christian Franke via Cygwin wrote:
>> Testcase:
>>
>> # cygcheck -f /sbin/fdisk.exe
>> util-linux-2.39.3-1
>>
>> # /sbin/fdisk.exe -l /dev/sdd
>> Disk /dev/sdd: 465.76 GiB, 500107862016 bytes, 976773168 sectors
>> Units: sectors of 1 * 512 = 512 bytes
>> Sector size (logical/physical): 512 bytes / 34359738880 bytes
>> I/O size (minimum/optimal): 34359738880 bytes / 34359738880 bytes
> [...valuable investigation and patch suggestion elided...]
>
> Your suggested patch looks fine to me.  I have added it to the patch 
> deck for a new util-linux 2.39.3-2, which has just been uploaded.  The 
> patch allows fdisk.exe to report the three correct values in my 
> limited testing.
> Thanks for the report and the patch!

You're welcome.

BTW, according to the Linux kernel sources, BLKPBSZGET etc return 
'unsigned int' and not 'unsigned long' since first appearance in 
2.6.32-rc3 (2009?):

https://elixir.bootlin.com/linux/v2.6.32-rc3/source/block/ioctl.c#L276
https://elixir.bootlin.com/linux/v2.6.32-rc3/source/block/compat_ioctl.c#L743
https://elixir.bootlin.com/linux/v6.8.2/source/block/ioctl.c#L533

So I don't understand why the mentioned code would be correct for Linux.

Re: util-linux-2.39.3-1: libblkid returns invalid physical_sector_size

[Christian Franke]

Christian Franke via Cygwin wrote:
> ,,,
> BTW, according to the Linux kernel sources, BLKPBSZGET etc return 
> 'unsigned int' and not 'unsigned long' since first appearance in 
> 2.6.32-rc3 (2009?):
>
> https://elixir.bootlin.com/linux/v2.6.32-rc3/source/block/ioctl.c#L276
> https://elixir.bootlin.com/linux/v2.6.32-rc3/source/block/compat_ioctl.c#L743 
>
> https://elixir.bootlin.com/linux/v6.8.2/source/block/ioctl.c#L533
>
> So I don't understand why the mentioned code would be correct for Linux.
>

It is likely an upstream regression from an 1+ year old commit. I filed 
a GH issue:
https://github.com/util-linux/util-linux/issues/2904

Re: util-linux-2.39.3-1: libblkid returns invalid physical_sector_size

[Mark Geisert]

On 4/2/2024 9:50 AM, Christian Franke via Cygwin wrote:
> Christian Franke via Cygwin wrote:
>> ,,,
>> BTW, according to the Linux kernel sources, BLKPBSZGET etc return 
>> 'unsigned int' and not 'unsigned long' since first appearance in 
>> 2.6.32-rc3 (2009?):
>>
>> https://elixir.bootlin.com/linux/v2.6.32-rc3/source/block/ioctl.c#L276
>> https://elixir.bootlin.com/linux/v2.6.32-rc3/source/block/compat_ioctl.c#L743
>> https://elixir.bootlin.com/linux/v6.8.2/source/block/ioctl.c#L533
>>
>> So I don't understand why the mentioned code would be correct for Linux.
>>
> 
> It is likely an upstream regression from an 1+ year old commit. I filed 
> a GH issue:
> https://github.com/util-linux/util-linux/issues/2904

Thank you Christian for reporting the issue upstream.  I won't be able 
to try out the proposed fix in that issue 2904 as I'm about to be AFK 
for two weeks plus.  I will check in upon my return to keyboard.
Cheers & Regards,

..mark

Re: util-linux-2.39.3-1: libblkid returns invalid physical_sector_size

[Christian Franke]

Bruce Jerrick via Cygwin wrote:
>> Downgrading to util-linux-2.33.3-3 does not help. The related code
>> differs, but has the same problem.

I take that back. The above should read "util-linux-2.33.1-3".


> But it was OK in util-linux-2.33.1-3 .

Yes, this is correct. I possibly downgraded util-linux, but forgot 
libblkid1.

Re: util-linux-2.39.3-1: libblkid returns invalid physical_sector_size

[Bruce Jerrick]

> Downgrading to util-linux-2.33.3-3 does not help. The related code
> differs, but has the same problem.

But it was OK in util-linux-2.33.1-3 .  The only difference in output
between that and the fixed 2.39.3-2 is that the latter includes one more
decimal place in the reported "human" sizes (GiB, TiB), which is very
welcome.