Re: [f2fs-dev] [PATCH v1] mkfs.f2fs: fix incorrect start_sector detection due to typo in HDIO_GETGEO macro check

[Chao Yu <ch...@ke...>]

On 11/19/2025 2:00 PM, Xiaole He wrote:
> The code was checking for a non-existent macro `HDIO_GETGIO` instead of
> `HDIO_GETGEO`, which caused the ioctl() call to never be executed.
> This resulted in start_sector always being set to 0, even when the
> partition actually starts at a different sector (e.g., 2048).
> 
> This bug affects:
> - lib/libf2fs.c: get_device_info() function (lines 967, 1076)
> - tools/fibmap.c: stat_bdev() function (lines 91, 108)
> 
> Reproducer:
> 1. Create a partition starting at sector 2048:
>     # parted -s /dev/sdb mklabel gpt
>     # parted -s /dev/sdb mkpart primary 2048s 32GiB
> 
> 2. Verify the partition start sector:
>     # cat /sys/block/sdb/sdb1/start
>     2048
> 
> 3. Format with f2fs-tools (before fix) and verify incorrect calculation:
>     # mkfs.f2fs /dev/sdb1
>     # Info: zone aligned segment0 blkaddr: 512
> 
>     The calculation happens in
>     mkfs/f2fs_format.c:f2fs_prepare_super_block():
> 
>     With default values:
>     - blk_size_bytes = 4096
>     - c.blks_per_seg = 512
>     - c.segs_per_sec = 1
>     - c.secs_per_zone = 1
>     - zone_size_bytes = blk_size_bytes * c.secs_per_zone
>                         * c.segs_per_sec * c.blks_per_seg
>                       = 4096 * 1 * 1 * 512 = 2,097,152 bytes (2MB)
>     - alignment_bytes = zone_size_bytes (for non-zoned or single device)
> 
>     With c.start_sector = 0 (bug case):
>     - zone_align_start_offset
>         = ((c.start_sector * DEFAULT_SECTOR_SIZE +
>            2 * F2FS_BLKSIZE + alignment_bytes - 1) /
>            alignment_bytes * alignment_bytes) -
>            (c.start_sector * DEFAULT_SECTOR_SIZE)
>         = ((0 * 512 + 2 * 4096 + 2,097,152 - 1) /
>            2,097,152 * 2,097,152) - (0 * 512)
>         = ((8,192 + 2,097,152 - 1) / 2,097,152 * 2,097,152) - 0
>         = 2,097,152 - 0 = 2,097,152 bytes (2MB)
>     - segment0_blkaddr = zone_align_start_offset / blk_size_bytes
>         = 2,097,152 / 4,096 = 512 blocks
> 
>     This matches the output "segment0 blkaddr: 512".
> 
>     With c.start_sector = 2048 (correct case):
>     - zone_align_start_offset
>         = ((2048 * 512 + 2 * 4096 + 2,097,152 - 1) /
>            2,097,152 * 2,097,152) - (2048 * 512)
>         = ((1,048,576 + 8,192 + 2,097,152 - 1) /
>            2,097,152 * 2,097,152) - 1,048,576
>         = 2,097,152 - 1,048,576 = 1,048,576 bytes (1MB)
>     - segment0_blkaddr = zone_align_start_offset / blk_size_bytes
>         = 1,048,576 / 4,096 = 256 blocks
> 
> 4. Confirm the bug with gdb:
>     So the correct output should be "segment0 blkaddr: 256", not 512.
> 
>     # gdb --args mkfs.f2fs /dev/sdb1
>     (gdb) b lib/libf2fs.c:1075
>     (gdb) run
>     1075                    if (i == 0) {
>     (gdb) l
>     1075                    if (i == 0) {
>     1076    #ifdef HDIO_GETGIO
>     1077                            if (ioctl(fd, HDIO_GETGEO, &geom) < 0)
>     1078                                    c.start_sector = 0;
>     1079                            else
>     1080                                    c.start_sector = geom.start;
>     1081    #else
>     1082                            c.start_sector = 0;
>     1083    #endif
>     1084                    }
>     (gdb) n
>     1082                            c.start_sector = 0;
>     # At line 1076: #ifdef HDIO_GETGIO (bug - this macro doesn't exist)
>     # The ioctl() call at line 1077 is skipped, and execution goes
>     # directly to line 1082, setting c.start_sector = 0
> 
> After fix:
>     mkfs.f2fs output:
>     Info: zone aligned segment0 blkaddr: 256
> 
>     gdb output:
>     (gdb) p c.start_sector
>     $1 = 2048
> 
>     The ioctl() correctly retrieves the partition start sector (2048),
>     and zone_align_start_offset is calculated correctly, resulting in
>     segment0_blkaddr = 256.
> 
> Fixes: 43bb7b6c99fa ("f2fs-tools: build binaries in Mac")
> Signed-off-by: Xiaole He <hex...@12...>

Nice catch and analysis!

Reviewed-by: Chao Yu <ch...@ke...>

Thanks,