Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 11 Jan 2020 17:41:20 +0000 (UTC)
From:      Xin LI <delphij@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r356636 - head/sbin/fsck_msdosfs
Message-ID:  <202001111741.00BHfK23099466@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: delphij
Date: Sat Jan 11 17:41:20 2020
New Revision: 356636
URL: https://svnweb.freebsd.org/changeset/base/356636

Log:
  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.
  
  PR:		243179
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D23082

Modified:
  head/sbin/fsck_msdosfs/boot.c

Modified: head/sbin/fsck_msdosfs/boot.c
==============================================================================
--- head/sbin/fsck_msdosfs/boot.c	Sat Jan 11 16:28:35 2020	(r356635)
+++ head/sbin/fsck_msdosfs/boot.c	Sat Jan 11 17:41:20 2020	(r356636)
@@ -266,8 +266,11 @@ readboot(int dosfs, struct bootblock *boot)
 		return FSFATAL;
 	}
 
-	boot->NumClusters = (boot->NumSectors - boot->FirstCluster) / boot->bpbSecPerClust +
-	    CLUST_FIRST;
+	/*
+	 * The number of clusters is derived from available data sectors, divided
+	 * by sectors per cluster.
+	 */
+	boot->NumClusters = (boot->NumSectors - boot->FirstCluster) / boot->bpbSecPerClust;
 
 	if (boot->flags & FAT32) {
 		if (boot->NumClusters > (CLUST_RSRVD & CLUST32_MASK)) {
@@ -310,11 +313,19 @@ readboot(int dosfs, struct bootblock *boot)
 		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
+	 * when 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->bpbBytesPerSec * boot->bpbSecPerClust;
 
 	boot->NumFiles = 1;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202001111741.00BHfK23099466>