Skip to content

Commit

Permalink
sbin/fsck_msdosfs: Fix an integer overflow on 32-bit platforms.
Browse files Browse the repository at this point in the history
The purpose of checksize() is to verify that the referenced cluster
chain size matches the recorded file size (up to 2^32 - 1) in the
directory entry. We follow the cluster chain, then multiple the
cluster count by bytes per cluster to get the physical size, then
check it against the recorded size.

When a file is close to 4 GiB (between 4GiB - cluster size and 4GiB,
both non-inclusive), the product of cluster count and bytes per
cluster would be exactly 4 GiB. On 32-bit systems, because size_t
is 32-bit, this would wrap back to 0, which will cause the file be
truncated to 0.

Fix this by using 64-bit physicalSize instead.

This fix is inspired by an Android change request at
https://android-review.googlesource.com/c/platform/external/fsck_msdos/+/1428461

PR:		249533
Reviewed by:	kevlo
MFC after:	3 days
Differential Revision:	https://reviews.freebsd.org/D26524
  • Loading branch information
delphij committed Sep 23, 2020
1 parent 00a2dc8 commit 4b0089a
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions sbin/fsck_msdosfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,8 @@ static int
checksize(struct fat_descriptor *fat, u_char *p, struct dosDirEntry *dir)
{
int ret = FSOK;
size_t physicalSize;
size_t chainsize;
u_int64_t physicalSize;
struct bootblock *boot;

boot = fat_get_boot(fat);
Expand All @@ -401,9 +402,9 @@ checksize(struct fat_descriptor *fat, u_char *p, struct dosDirEntry *dir)
} else {
if (!fat_is_valid_cl(fat, dir->head))
return FSERROR;
ret = checkchain(fat, dir->head, &physicalSize);
ret = checkchain(fat, dir->head, &chainsize);
/*
* Upon return, physicalSize would hold the chain length
* Upon return, chainsize would hold the chain length
* that checkchain() was able to validate, but if the user
* refused the proposed repair, it would be unsafe to
* proceed with directory entry fix, so bail out in that
Expand All @@ -412,7 +413,13 @@ checksize(struct fat_descriptor *fat, u_char *p, struct dosDirEntry *dir)
if (ret == FSERROR) {
return (FSERROR);
}
physicalSize *= boot->ClusterSize;
/*
* The maximum file size on FAT32 is 4GiB - 1, which
* will occupy a cluster chain of exactly 4GiB in
* size. On 32-bit platforms, since size_t is 32-bit,
* it would wrap back to 0.
*/
physicalSize = (u_int64_t)chainsize * boot->ClusterSize;
}
if (physicalSize < dir->size) {
pwarn("size of %s is %u, should at most be %zu\n",
Expand Down

0 comments on commit 4b0089a

Please sign in to comment.