[src/trunk]: src/sbin/fsck_msdos Don't add the 2 reserved clusters before we ...

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/c4c084d3abbf
branches:  trunk
changeset: 847930:c4c084d3abbf
user:      christos <christos%NetBSD.org@localhost>
date:      Sat Jan 11 16:29:07 2020 +0000

description:
Don't add the 2 reserved clusters before we determine if we using fat16/fat32.
>From FreeBSD: https://reviews.freebsd.org/D23082:

Correct off-by-two issue when determining FAT type.

In the code we used NumClusters as the upper (non-inclusive) boundary
of valid cluster number, so the actual value was 2 (CLUST_FIRST) more
than the real number of clusters. This causes a FAT16 media with
65524 clusters be treated as FAT32 and might affect FAT12 media with
4084 clusters as well.

To fix this, we increment NumClusters by CLUST_FIRST after the type
determination.

diffstat:

 sbin/fsck_msdos/boot.c |  20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

diffs (48 lines):

diff -r 803e6c732e0e -r c4c084d3abbf sbin/fsck_msdos/boot.c
--- a/sbin/fsck_msdos/boot.c    Sat Jan 11 04:53:10 2020 +0000
+++ b/sbin/fsck_msdos/boot.c    Sat Jan 11 16:29:07 2020 +0000
@@ -27,7 +27,7 @@
 
 #include <sys/cdefs.h>
 #ifndef lint
-__RCSID("$NetBSD: boot.c,v 1.21 2018/02/08 09:05:17 dholland Exp $");
+__RCSID("$NetBSD: boot.c,v 1.22 2020/01/11 16:29:07 christos Exp $");
 #endif /* not lint */
 
 #include <stdlib.h>
@@ -210,8 +210,12 @@
                return FSFATAL;
        }
 
-       boot->NumClusters = (boot->NumSectors - boot->FirstCluster) / boot->SecPerClust
-                           + CLUST_FIRST;
+       /*
+        * The number of clusters is derived from available data sectors,
+        * divided by sectors per cluster.
+        */
+       boot->NumClusters =
+           (boot->NumSectors - boot->FirstCluster) / boot->SecPerClust;
 
        if (boot->flags&FAT32)
                boot->ClustMask = CLUST32_MASK;
@@ -237,11 +241,19 @@
                break;
        }
 
-       if (boot->NumFatEntries < boot->NumClusters - CLUST_FIRST) {
+       if (boot->NumFatEntries < boot->NumClusters) {
                pfatal("FAT size too small, %u entries won't fit into %u sectors\n",
                       boot->NumClusters, boot->FATsecs);
                return FSFATAL;
        }
+
+       /*
+        * There are two reserved clusters. To avoid adding CLUST_FIRST every
+        * time we perform boundary checks, we increment the NumClusters by 2,
+        * which is CLUST_FIRST to denote the first out-of-range cluster number.
+        */
+       boot->NumClusters += CLUST_FIRST;
+
        boot->ClusterSize = boot->BytesPerSec * boot->SecPerClust;
 
        boot->NumFiles = 1;