problem with exfat on 4k logical sector devices

problem with exfat on 4k logical sector devices

[Eric Sandeen]

Hi Namjae - 

It seems that exfat is unhappy on 4k logical sector size devices:

[root@big18 exfatprogs]# modprobe scsi_debug sector_size=4096 dev_size_mb=256
[root@big18 exfatprogs]# dmesg | tail -n 1
[933449.931608] sd 16:0:0:0: [sdh] Attached SCSI disk
[root@big18 exfatprogs]# mkfs.exfat /dev/sdh
exfatprogs version : 1.0.4
Creating exFAT filesystem(/dev/sdh, cluster size=4096)

Writing volume boot record: done
Writing backup volume boot record: done
Fat table creation: done
Allocation bitmap creation: done
Upcase table creation: done
Writing root directory entry: done
Synchronizing...

exFAT format complete!

[root@big18 exfatprogs]# fsck.exfat /dev/sdh
exfatprogs version : 1.0.4
checksums of boot sector are not correct. 0x44e6c5, but expected 0xda55694. Fix (y/N)? n

[root@big18 exfatprogs]# mount /dev/sdh /mnt/test
mount: /mnt/test: wrong fs type, bad option, bad superblock on /dev/sdh, missing codepage or helper program, or other error.
[root@big18 exfatprogs]# dmesg
<...>
[933485.685102] exFAT-fs (sdh): Invalid exboot-signature(sector = 1): 0x00000000
[933485.686134] exFAT-fs (sdh): Invalid exboot-signature(sector = 2): 0x00000000
[933485.687173] exFAT-fs (sdh): Invalid exboot-signature(sector = 3): 0x00000000
[933485.688209] exFAT-fs (sdh): Invalid exboot-signature(sector = 4): 0x00000000
[933485.689247] exFAT-fs (sdh): Invalid exboot-signature(sector = 5): 0x00000000
[933485.690283] exFAT-fs (sdh): Invalid exboot-signature(sector = 6): 0x00000000
[933485.691318] exFAT-fs (sdh): Invalid exboot-signature(sector = 7): 0x00000000
[933485.692352] exFAT-fs (sdh): Invalid exboot-signature(sector = 8): 0x00000000
[933485.695450] exFAT-fs (sdh): Invalid boot checksum (boot checksum : 0x0044e6c5, checksum : 0x04653cbf)
[933485.695452] exFAT-fs (sdh): invalid boot region
[933485.695453] exFAT-fs (sdh): failed to recognize exfat type

I think the primary problem here is that the boot sector disk structures are always 512 bytes, even if the underlying disk sectors are 4k. There is a mismatch between mkfs, fsck, and the kernel in this respect. mkfs calculates checksums using 512 bytes for all but the OEM and reserved sectors, where it uses sector_size instead (which may be 4k)

3 mkfs/mkfs.c    exfat_write_boot_sector            129 boot_calc_checksum((unsigned char *)ppbr, sizeof(struct pbr),
4 mkfs/mkfs.c    exfat_write_extended_boot_sectors  155 boot_calc_checksum((unsigned char *) &eb, sizeof(struct exbs),
5 mkfs/mkfs.c    exfat_write_oem_sector             184 boot_calc_checksum((unsigned char *)oem, bd->sector_size, false,
6 mkfs/mkfs.c    exfat_write_oem_sector             196 boot_calc_checksum((unsigned char *)oem, bd->sector_size, false,

but fsck uses the disk sector size (4k) for everything, so there is a checksum mismatch and failure.

exfat_update_boot_checksum()
...
        int sector_size = bd->sector_size;
...
                boot_calc_checksum(buf, sector_size, is_boot_sec,
                        &checksum);

The kernel has similar problems at mount time.

(the kernel also has an issue where exfat_verify_boot_region is looking at 4 bytes from the end of the sector for EXBOOT_SIGNATURE, rather than 4 bytes from the end of the boot_sector.  Also, s_blocksize_bits never gets set...)

Anyway, this can all be fixed, but first there is a question about what is proper:

For these 11 regions (main boot sector, main extended boot sectors, OEM, and reserved, I think?) should the checksums be calculated on the full sector size (possibly 4k) or 512, or is it calculated on 512 (structure size) for all but the OEM sector as mkfs does? It's not clear to me from a quick read of the spec.

(But from the example at https://docs.microsoft.com/en-us/windows/win32/fileio/exfat-specification#figure-1-boot-checksum-computation it almost looks like the checksum calculation should cover the entire 4k for all these regions, even if the disk structure itself is smaller?)

Thanks,
-Eric

Re: problem with exfat on 4k logical sector devices

[Namjae Jeon]

> Hi Namjae -
Hi Eric,
>
> It seems that exfat is unhappy on 4k logical sector size devices:
Thanks for your report!
We have got same report from Christophe Vu-Brugier. And he sent us
the patch(https://github.com/exfatprogs/exfatprogs/pull/164) to fix it
yesterday.(Thanks Christophe!), I will check it today.
>
> [root@big18 exfatprogs]# modprobe scsi_debug sector_size=4096
> dev_size_mb=256
> [root@big18 exfatprogs]# dmesg | tail -n 1
> [933449.931608] sd 16:0:0:0: [sdh] Attached SCSI disk
> [root@big18 exfatprogs]# mkfs.exfat /dev/sdh
> exfatprogs version : 1.0.4
> Creating exFAT filesystem(/dev/sdh, cluster size=4096)
>
> Writing volume boot record: done
> Writing backup volume boot record: done
> Fat table creation: done
> Allocation bitmap creation: done
> Upcase table creation: done
> Writing root directory entry: done
> Synchronizing...
>
> exFAT format complete!
>
> [root@big18 exfatprogs]# fsck.exfat /dev/sdh
> exfatprogs version : 1.0.4
> checksums of boot sector are not correct. 0x44e6c5, but expected 0xda55694.
> Fix (y/N)? n
>
> [root@big18 exfatprogs]# mount /dev/sdh /mnt/test
> mount: /mnt/test: wrong fs type, bad option, bad superblock on /dev/sdh,
> missing codepage or helper program, or other error.
> [root@big18 exfatprogs]# dmesg
> <...>
> [933485.685102] exFAT-fs (sdh): Invalid exboot-signature(sector = 1):
> 0x00000000
> [933485.686134] exFAT-fs (sdh): Invalid exboot-signature(sector = 2):
> 0x00000000
> [933485.687173] exFAT-fs (sdh): Invalid exboot-signature(sector = 3):
> 0x00000000
> [933485.688209] exFAT-fs (sdh): Invalid exboot-signature(sector = 4):
> 0x00000000
> [933485.689247] exFAT-fs (sdh): Invalid exboot-signature(sector = 5):
> 0x00000000
> [933485.690283] exFAT-fs (sdh): Invalid exboot-signature(sector = 6):
> 0x00000000
> [933485.691318] exFAT-fs (sdh): Invalid exboot-signature(sector = 7):
> 0x00000000
> [933485.692352] exFAT-fs (sdh): Invalid exboot-signature(sector = 8):
> 0x00000000
> [933485.695450] exFAT-fs (sdh): Invalid boot checksum (boot checksum :
> 0x0044e6c5, checksum : 0x04653cbf)
> [933485.695452] exFAT-fs (sdh): invalid boot region
> [933485.695453] exFAT-fs (sdh): failed to recognize exfat type
>
> I think the primary problem here is that the boot sector disk structures are
> always 512 bytes, even if the underlying disk sectors are 4k. There is a
> mismatch between mkfs, fsck, and the kernel in this respect. mkfs calculates
> checksums using 512 bytes for all but the OEM and reserved sectors, where it
> uses sector_size instead (which may be 4k)
Right.
>
> 3 mkfs/mkfs.c    exfat_write_boot_sector            129
> boot_calc_checksum((unsigned char *)ppbr, sizeof(struct pbr),
> 4 mkfs/mkfs.c    exfat_write_extended_boot_sectors  155
> boot_calc_checksum((unsigned char *) &eb, sizeof(struct exbs),
> 5 mkfs/mkfs.c    exfat_write_oem_sector             184
> boot_calc_checksum((unsigned char *)oem, bd->sector_size, false,
> 6 mkfs/mkfs.c    exfat_write_oem_sector             196
> boot_calc_checksum((unsigned char *)oem, bd->sector_size, false,
>
> but fsck uses the disk sector size (4k) for everything, so there is a
> checksum mismatch and failure.
>
> exfat_update_boot_checksum()
> ...
>         int sector_size = bd->sector_size;
> ...
>                 boot_calc_checksum(buf, sector_size, is_boot_sec,
>                         &checksum);
>
> The kernel has similar problems at mount time.
>
> (the kernel also has an issue where exfat_verify_boot_region is looking at 4
> bytes from the end of the sector for EXBOOT_SIGNATURE, rather than 4 bytes
> from the end of the boot_sector.  Also, s_blocksize_bits never gets set...)
>
> Anyway, this can all be fixed, but first there is a question about what is
> proper:
Okay.
>
> For these 11 regions (main boot sector, main extended boot sectors, OEM, and
> reserved, I think?) should the checksums be calculated on the full sector
> size (possibly 4k) or 512, or is it calculated on 512 (structure size) for
> all but the OEM sector as mkfs does? It's not clear to me from a quick read
> of the spec.
>
> (But from the example at
> https://docs.microsoft.com/en-us/windows/win32/fileio/exfat-specification#figure-1-boot-checksum-computation
> it almost looks like the checksum calculation should cover the entire 4k for
> all these regions, even if the disk structure itself is smaller?)
In this case,  We can check a dump of boot sectors formatted from Windows.
Let me check it more.

Thanks Eric!
>
> Thanks,
> -Eric
>

Re: problem with exfat on 4k logical sector devices

[Eric Sandeen]

On 5/11/21 4:21 PM, Namjae Jeon wrote:
>> Hi Namjae -
> Hi Eric,
>>
>> It seems that exfat is unhappy on 4k logical sector size devices:
> Thanks for your report!
> We have got same report from Christophe Vu-Brugier. And he sent us
> the patch(https://github.com/exfatprogs/exfatprogs/pull/164) to fix it
> yesterday.(Thanks Christophe!), I will check it today

Oh, good timing! ;)

gI'll try to look at that in more depth. It does seem to make everything
work for me, and resolves a couple other misunderstandings I may have had, and
they seem to match with the spec.

For example, I now see that boot sector signature does go at the end of 512 for
the primary boot sector, and at the end of $SECTOR_SIZE for the extended boot
sector.

One other thing that I ran across is that fsck seems to validate an image
against the sector size of the device hosting the image rather than the sector size
found in the boot sector, which seems like another issue that will come up:

# fsck/fsck.exfat /dev/sdb
exfatprogs version : 1.1.1
/dev/sdb: clean. directories 1, files 0

# dd if=/dev/sdb of=test.img
524288+0 records in
524288+0 records out
268435456 bytes (268 MB) copied, 1.27619 s, 210 MB/s

# fsck.exfat test.img 
exfatprogs version : 1.1.1
checksum of boot region is not correct. 0, but expected 0x3ee721
boot region is corrupted. try to restore the region from backup. Fix (y/N)? n

Right now the utilities seem to assume that the device they're pointed at is
always a block device, and image files are problematic.

Also, as an aside, it might be useful to have a "set sector size" commandline
option at least for testing, or to create 4k images that could be transferred
to a 4k device.

Thanks,
-Eric

RE: problem with exfat on 4k logical sector devices

[Namjae Jeon]

> On 5/11/21 4:21 PM, Namjae Jeon wrote:
> >> Hi Namjae -
> > Hi Eric,
> >>
> >> It seems that exfat is unhappy on 4k logical sector size devices:
> > Thanks for your report!
> > We have got same report from Christophe Vu-Brugier. And he sent us the
> > patch(https://protect2.fireeye.com/v1/url?k=ac8f77ef-f3144ef5-ac8efca0
> > -000babff24ad-8b7be88b031de920&q=1&e=0e9634f8-7ff9-4eb8-b5af-2316b62e9
> > 236&u=https%3A%2F%2Fgithub.com%2Fexfatprogs%2Fexfatprogs%2Fpull%2F164)
> > to fix it yesterday.(Thanks Christophe!), I will check it today
> 
> Oh, good timing! ;)
> 
> gI'll try to look at that in more depth. It does seem to make everything work for me, and resolves a
> couple other misunderstandings I may have had, and they seem to match with the spec.
> 
> For example, I now see that boot sector signature does go at the end of 512 for the primary boot
> sector, and at the end of $SECTOR_SIZE for the extended boot sector.
Thanks for your check:)

> 
> One other thing that I ran across is that fsck seems to validate an image against the sector size of
> the device hosting the image rather than the sector size found in the boot sector, which seems like
> another issue that will come up:
> 
> # fsck/fsck.exfat /dev/sdb
> exfatprogs version : 1.1.1
> /dev/sdb: clean. directories 1, files 0
> 
> # dd if=/dev/sdb of=test.img
> 524288+0 records in
> 524288+0 records out
> 268435456 bytes (268 MB) copied, 1.27619 s, 210 MB/s
> 
> # fsck.exfat test.img
> exfatprogs version : 1.1.1
> checksum of boot region is not correct. 0, but expected 0x3ee721 boot region is corrupted. try to
> restore the region from backup. Fix (y/N)? n
> 
> Right now the utilities seem to assume that the device they're pointed at is always a block device,
> and image files are problematic.
Okay, Will fix it.
> 
> Also, as an aside, it might be useful to have a "set sector size" commandline option at least for
> testing, or to create 4k images that could be transferred to a 4k device.
Agreed, We will add that option:)

Thanks!
> 
> Thanks,
> -Eric

Re: problem with exfat on 4k logical sector devices

[Eric Sandeen]

On 5/11/21 6:53 PM, Namjae Jeon wrote:

>> One other thing that I ran across is that fsck seems to validate an image against the sector size of
>> the device hosting the image rather than the sector size found in the boot sector, which seems like
>> another issue that will come up:
>>
>> # fsck/fsck.exfat /dev/sdb
>> exfatprogs version : 1.1.1
>> /dev/sdb: clean. directories 1, files 0
>>
>> # dd if=/dev/sdb of=test.img
>> 524288+0 records in
>> 524288+0 records out
>> 268435456 bytes (268 MB) copied, 1.27619 s, 210 MB/s
>>
>> # fsck.exfat test.img
>> exfatprogs version : 1.1.1
>> checksum of boot region is not correct. 0, but expected 0x3ee721 boot region is corrupted. try to
>> restore the region from backup. Fix (y/N)? n
>>
>> Right now the utilities seem to assume that the device they're pointed at is always a block device,
>> and image files are problematic.
> Okay, Will fix it.

Right now I have a hack like this.

1) don't validate the in-image sector size against the host device size
(maybe should only skip this check if it's not a bdev? Or is it OK to have
a 4k sector size fs on a 512 device? Probably?)

2) populate the "bd" sector size information from the values read from the image.

It feels a bit messy, but it works so far. I guess the messiness stems from
assuming that we always have a "bd" block device.

-Eric

diff --git a/dump/dump.c b/dump/dump.c
index 85d5101..30ec8cb 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -100,6 +100,9 @@ static int exfat_show_ondisk_all_info(struct exfat_blk_dev *bd)
 		goto free_ppbr;
 	}
 
+	bd->sector_size_bits = pbsx->sect_size_bits;
+	bd->sector_size = 1 << pbsx->sect_size_bits;
+
 	if (pbsx->sect_per_clus_bits > 25 - pbsx->sect_size_bits) {
 		exfat_err("bogus sectors bits per cluster : %u\n",
 				pbsx->sect_per_clus_bits);
@@ -107,13 +110,6 @@ static int exfat_show_ondisk_all_info(struct exfat_blk_dev *bd)
 		goto free_ppbr;
 	}
 
-	if (bd->sector_size != 1 << pbsx->sect_size_bits) {
-		exfat_err("bogus sector size : %u (sector size bits : %u)\n",
-				bd->sector_size, pbsx->sect_size_bits);
-		ret = -EINVAL;
-		goto free_ppbr;
-	}
-
 	clu_offset = le32_to_cpu(pbsx->clu_offset);
 	total_clus = le32_to_cpu(pbsx->clu_count);
 	root_clu = le32_to_cpu(pbsx->root_cluster);
diff --git a/fsck/fsck.c b/fsck/fsck.c
index 747a771..5ea8278 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -682,6 +682,9 @@ static int read_boot_region(struct exfat_blk_dev *bd, struct pbr **pbr,
 		goto err;
 	}
 
+	bd->sector_size_bits = bs->bsx.sect_size_bits;
+	bd->sector_size = 1 << bs->bsx.sect_size_bits;
+
 	ret = boot_region_checksum(bd, bs_offset);
 	if (ret < 0)
 		goto err;

Re: problem with exfat on 4k logical sector devices

[Hyunchul Lee]

Hello,

2021년 5월 12일 (수) 오전 8:57, Eric Sandeen <sandeen@sandeen.net>님이 작성:
>
> On 5/11/21 6:53 PM, Namjae Jeon wrote:
>
> >> One other thing that I ran across is that fsck seems to validate an image against the sector size of
> >> the device hosting the image rather than the sector size found in the boot sector, which seems like
> >> another issue that will come up:
> >>
> >> # fsck/fsck.exfat /dev/sdb
> >> exfatprogs version : 1.1.1
> >> /dev/sdb: clean. directories 1, files 0
> >>
> >> # dd if=/dev/sdb of=test.img
> >> 524288+0 records in
> >> 524288+0 records out
> >> 268435456 bytes (268 MB) copied, 1.27619 s, 210 MB/s
> >>
> >> # fsck.exfat test.img
> >> exfatprogs version : 1.1.1
> >> checksum of boot region is not correct. 0, but expected 0x3ee721 boot region is corrupted. try to
> >> restore the region from backup. Fix (y/N)? n
> >>
> >> Right now the utilities seem to assume that the device they're pointed at is always a block device,
> >> and image files are problematic.
> > Okay, Will fix it.
>
> Right now I have a hack like this.
>
> 1) don't validate the in-image sector size against the host device size
> (maybe should only skip this check if it's not a bdev? Or is it OK to have
> a 4k sector size fs on a 512 device? Probably?)
>
> 2) populate the "bd" sector size information from the values read from the image.
>
> It feels a bit messy, but it works so far. I guess the messiness stems from
> assuming that we always have a "bd" block device.
>

I think we need to keep the "bd" sector size to avoid confusion between
the device's sector size and the exfat's sector size.

> -Eric
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 85d5101..30ec8cb 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -100,6 +100,9 @@ static int exfat_show_ondisk_all_info(struct exfat_blk_dev *bd)
>                 goto free_ppbr;
>         }
>
> +       bd->sector_size_bits = pbsx->sect_size_bits;
> +       bd->sector_size = 1 << pbsx->sect_size_bits;
> +
>         if (pbsx->sect_per_clus_bits > 25 - pbsx->sect_size_bits) {
>                 exfat_err("bogus sectors bits per cluster : %u\n",
>                                 pbsx->sect_per_clus_bits);
> @@ -107,13 +110,6 @@ static int exfat_show_ondisk_all_info(struct exfat_blk_dev *bd)
>                 goto free_ppbr;
>         }
>
> -       if (bd->sector_size != 1 << pbsx->sect_size_bits) {
> -               exfat_err("bogus sector size : %u (sector size bits : %u)\n",
> -                               bd->sector_size, pbsx->sect_size_bits);
> -               ret = -EINVAL;
> -               goto free_ppbr;
> -       }
> -
>         clu_offset = le32_to_cpu(pbsx->clu_offset);
>         total_clus = le32_to_cpu(pbsx->clu_count);
>         root_clu = le32_to_cpu(pbsx->root_cluster);
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 747a771..5ea8278 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -682,6 +682,9 @@ static int read_boot_region(struct exfat_blk_dev *bd, struct pbr **pbr,
>                 goto err;
>         }
>
> +       bd->sector_size_bits = bs->bsx.sect_size_bits;
> +       bd->sector_size = 1 << bs->bsx.sect_size_bits;
> +

Is it better to add a sector size parameter to read_boot_region
function? This function
is also called to read the backup boot region to restore the corrupted
main boot region.
During this restoration, we need to read the backup boot region with
various sector sizes,
Because we don't have a certain exfat sector size.

>         ret = boot_region_checksum(bd, bs_offset);
>         if (ret < 0)
>                 goto err;
>
>

I sent the pull request to fix these problems. Could you check this request?
https://github.com/exfatprogs/exfatprogs/pull/167

Thanks,
Hyunchul.

Re: problem with exfat on 4k logical sector devices

[Eric Sandeen]

On 5/12/21 9:09 AM, Hyunchul Lee wrote:
> Hello,
> 
> 2021년 5월 12일 (수) 오전 8:57, Eric Sandeen <sandeen@sandeen.net>님이 작성:
>>
>> On 5/11/21 6:53 PM, Namjae Jeon wrote:
>>
>>>> One other thing that I ran across is that fsck seems to validate an image against the sector size of
>>>> the device hosting the image rather than the sector size found in the boot sector, which seems like
>>>> another issue that will come up:
>>>>
>>>> # fsck/fsck.exfat /dev/sdb
>>>> exfatprogs version : 1.1.1
>>>> /dev/sdb: clean. directories 1, files 0
>>>>
>>>> # dd if=/dev/sdb of=test.img
>>>> 524288+0 records in
>>>> 524288+0 records out
>>>> 268435456 bytes (268 MB) copied, 1.27619 s, 210 MB/s
>>>>
>>>> # fsck.exfat test.img
>>>> exfatprogs version : 1.1.1
>>>> checksum of boot region is not correct. 0, but expected 0x3ee721 boot region is corrupted. try to
>>>> restore the region from backup. Fix (y/N)? n
>>>>
>>>> Right now the utilities seem to assume that the device they're pointed at is always a block device,
>>>> and image files are problematic.
>>> Okay, Will fix it.
>>
>> Right now I have a hack like this.
>>
>> 1) don't validate the in-image sector size against the host device size
>> (maybe should only skip this check if it's not a bdev? Or is it OK to have
>> a 4k sector size fs on a 512 device? Probably?)
>>
>> 2) populate the "bd" sector size information from the values read from the image.
>>
>> It feels a bit messy, but it works so far. I guess the messiness stems from
>> assuming that we always have a "bd" block device.
>>
> 
> I think we need to keep the "bd" sector size to avoid confusion between
> the device's sector size and the exfat's sector size.

Sure, it's just that for a filesystem in an image file, there is no meaning to
the "device sector size" because there is no device.

...

> Is it better to add a sector size parameter to read_boot_region
> function? This function
> is also called to read the backup boot region to restore the corrupted
> main boot region.
> During this restoration, we need to read the backup boot region with
> various sector sizes,
> Because we don't have a certain exfat sector size.
> 
>>         ret = boot_region_checksum(bd, bs_offset);
>>         if (ret < 0)
>>                 goto err;
>>
>>
> 
> I sent the pull request to fix these problems. Could you check this request?
> https://github.com/exfatprogs/exfatprogs/pull/167

I didn't review that in depth, but it looks like for fsck and dump, it gets the
sector size from the boot sector rather than from the host device or filesystem,
which makes sense, at least.

(As an aside, I'd suggest that your new "c2o" function could have a more
descriptive, self-documenting name.)

But there are other problems where bd->sector_* is used and assumed to relate
to the filesystem, for example:

# fsck/fsck.exfat /root/test.img 
exfatprogs version : 1.1.1
/root/test.img: clean. directories 1, files 0
# tune/tune.exfat -I 0x1234  /root/test.img 
exfatprogs version : 1.1.1
New volume serial : 0x1234
# fsck/fsck.exfat /root/test.img 
exfatprogs version : 1.1.1
checksum of boot region is not correct. 0x3eedc5, but expected 0xe59577e3
boot region is corrupted. try to restore the region from backup. Fix (y/N)? n

I think because exfat_write_checksum_sector() relies on bd->sector_size.

You probably need to audit every use of bd->sector_size and
bd->sector_size_bits outside of mkfs, because anything assuming that it
is related to the filesystem itself, as opposed to the filesytem/device
hosting it, could be problematic.  Is there any time outside of mkfs that
you actually care about the device sector size? If not, I might suggest
trying to isolate that usage to mkfs.

I also wonder about:

        bd->num_sectors = blk_dev_size / DEFAULT_SECTOR_SIZE;
        bd->num_clusters = blk_dev_size / ui->cluster_size;

is it really correct that this should always be in terms of 512-byte sectors?

-Eric

Re: problem with exfat on 4k logical sector devices

[Eric Sandeen]

On 5/12/21 11:44 AM, Eric Sandeen wrote:
> I also wonder about:
> 
>         bd->num_sectors = blk_dev_size / DEFAULT_SECTOR_SIZE;
>         bd->num_clusters = blk_dev_size / ui->cluster_size;
> 
> is it really correct that this should always be in terms of 512-byte sectors?

It does look like this causes problems as well:

# dump/dump.exfat /root/test.img 
exfatprogs version : 1.1.1
-------------- Dump Boot sector region --------------
Volume Length(sectors): 		65536 <<<<<<
FAT Offset(sector offset): 		256
FAT Length(sectors): 			64
Cluster Heap Offset (sector offset): 	512
Cluster Count: 				65024
Root Cluster (cluster offset): 		6
Volume Serial: 				0x1234
Sector Size Bits: 			12    <<<<<<<
Sector per Cluster bits: 		0

...

# tune/tune.exfat -v /root/test.img  
exfatprogs version : 1.1.1
[exfat_get_blk_dev_info: 202] Block device name : /root/test.img
[exfat_get_blk_dev_info: 203] Block device offset : 0
[exfat_get_blk_dev_info: 204] Block device size : 268435456
[exfat_get_blk_dev_info: 205] Block sector size : 512         <<<<<<<
[exfat_get_blk_dev_info: 206] Number of the sectors : 524288  <<<<<<<
[exfat_get_blk_dev_info: 208] Number of the clusters : 65536

RE: problem with exfat on 4k logical sector devices

[Namjae Jeon]

> 
> On 5/12/21 9:09 AM, Hyunchul Lee wrote:
> > Hello,
> >
> > 2021년 5월 12일 (수) 오전 8:57, Eric Sandeen <sandeen@sandeen.net>님이 작성:
> >>
> >> On 5/11/21 6:53 PM, Namjae Jeon wrote:
> >>
> >>>> One other thing that I ran across is that fsck seems to validate an
> >>>> image against the sector size of the device hosting the image
> >>>> rather than the sector size found in the boot sector, which seems like another issue that will
> come up:
> >>>>
> >>>> # fsck/fsck.exfat /dev/sdb
> >>>> exfatprogs version : 1.1.1
> >>>> /dev/sdb: clean. directories 1, files 0
> >>>>
> >>>> # dd if=/dev/sdb of=test.img
> >>>> 524288+0 records in
> >>>> 524288+0 records out
> >>>> 268435456 bytes (268 MB) copied, 1.27619 s, 210 MB/s
> >>>>
> >>>> # fsck.exfat test.img
> >>>> exfatprogs version : 1.1.1
> >>>> checksum of boot region is not correct. 0, but expected 0x3ee721
> >>>> boot region is corrupted. try to restore the region from backup.
> >>>> Fix (y/N)? n
> >>>>
> >>>> Right now the utilities seem to assume that the device they're
> >>>> pointed at is always a block device, and image files are problematic.
> >>> Okay, Will fix it.
> >>
> >> Right now I have a hack like this.
> >>
> >> 1) don't validate the in-image sector size against the host device
> >> size (maybe should only skip this check if it's not a bdev? Or is it
> >> OK to have a 4k sector size fs on a 512 device? Probably?)
> >>
> >> 2) populate the "bd" sector size information from the values read from the image.
> >>
> >> It feels a bit messy, but it works so far. I guess the messiness
> >> stems from assuming that we always have a "bd" block device.
> >>
> >
> > I think we need to keep the "bd" sector size to avoid confusion
> > between the device's sector size and the exfat's sector size.
> 
> Sure, it's just that for a filesystem in an image file, there is no meaning to the "device sector
> size" because there is no device.
> 
> ...
> 
> > Is it better to add a sector size parameter to read_boot_region
> > function? This function is also called to read the backup boot region
> > to restore the corrupted main boot region.
> > During this restoration, we need to read the backup boot region with
> > various sector sizes, Because we don't have a certain exfat sector
> > size.
> >
> >>         ret = boot_region_checksum(bd, bs_offset);
> >>         if (ret < 0)
> >>                 goto err;
> >>
> >>
> >
> > I sent the pull request to fix these problems. Could you check this request?
> > https://protect2.fireeye.com/v1/url?k=7932f7a1-26a9cef7-79337cee-0cc47
> > a31ce52-924b76d62e7bfc04&q=1&e=433c5d9e-f62a-4378-9b98-3c965d70e4da&u=
> > https%3A%2F%2Fgithub.com%2Fexfatprogs%2Fexfatprogs%2Fpull%2F167
> 
> I didn't review that in depth, but it looks like for fsck and dump, it gets the sector size from the
> boot sector rather than from the host device or filesystem, which makes sense, at least.
> 
> (As an aside, I'd suggest that your new "c2o" function could have a more descriptive, self-documenting
> name.)
> 
> But there are other problems where bd->sector_* is used and assumed to relate to the filesystem, for
> example:
> 
> # fsck/fsck.exfat /root/test.img
> exfatprogs version : 1.1.1
> /root/test.img: clean. directories 1, files 0 # tune/tune.exfat -I 0x1234  /root/test.img exfatprogs
> version : 1.1.1 New volume serial : 0x1234 # fsck/fsck.exfat /root/test.img exfatprogs version : 1.1.1
> checksum of boot region is not correct. 0x3eedc5, but expected 0xe59577e3 boot region is corrupted.
> try to restore the region from backup. Fix (y/N)? n
> 
> I think because exfat_write_checksum_sector() relies on bd->sector_size.
> 
> You probably need to audit every use of bd->sector_size and
> bd->sector_size_bits outside of mkfs, because anything assuming that it
> is related to the filesystem itself, as opposed to the filesytem/device hosting it, could be
> problematic.  Is there any time outside of mkfs that you actually care about the device sector size?
> If not, I might suggest trying to isolate that usage to mkfs.
I do not object to updating bd->sector_size to sector size obtained from boot sector.
I think that's the best way to do it.

> 
> I also wonder about:
> 
>         bd->num_sectors = blk_dev_size / DEFAULT_SECTOR_SIZE;
>         bd->num_clusters = blk_dev_size / ui->cluster_size;
> 
> is it really correct that this should always be in terms of 512-byte sectors?
Yep, Need to fix it.

Thanks!
> 
> -Eric

RE: problem with exfat on 4k logical sector devices

[Namjae Jeon]

> On 5/12/21 11:44 AM, Eric Sandeen wrote:
> > I also wonder about:
> >
> >         bd->num_sectors = blk_dev_size / DEFAULT_SECTOR_SIZE;
> >         bd->num_clusters = blk_dev_size / ui->cluster_size;
> >
> > is it really correct that this should always be in terms of 512-byte sectors?
> 
> It does look like this causes problems as well:
> 
> # dump/dump.exfat /root/test.img
> exfatprogs version : 1.1.1
> -------------- Dump Boot sector region --------------
> Volume Length(sectors): 		65536 <<<<<<
> FAT Offset(sector offset): 		256
> FAT Length(sectors): 			64
> Cluster Heap Offset (sector offset): 	512
> Cluster Count: 				65024
> Root Cluster (cluster offset): 		6
> Volume Serial: 				0x1234
> Sector Size Bits: 			12    <<<<<<<
> Sector per Cluster bits: 		0
> 
> ...
> 
> # tune/tune.exfat -v /root/test.img
> exfatprogs version : 1.1.1
> [exfat_get_blk_dev_info: 202] Block device name : /root/test.img
> [exfat_get_blk_dev_info: 203] Block device offset : 0
> [exfat_get_blk_dev_info: 204] Block device size : 268435456
> [exfat_get_blk_dev_info: 205] Block sector size : 512         <<<<<<<
> [exfat_get_blk_dev_info: 206] Number of the sectors : 524288  <<<<<<<
> [exfat_get_blk_dev_info: 208] Number of the clusters : 65536
Understood, Will fix it.
Thanks for detailed explanation!