Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 30 Apr 2020 06:34:34 +0000 (UTC)
From:      Xin LI <delphij@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r360490 - stable/11/sbin/fsck_msdosfs
Message-ID:  <202004300634.03U6YYT1049305@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: delphij
Date: Thu Apr 30 06:34:34 2020
New Revision: 360490
URL: https://svnweb.freebsd.org/changeset/base/360490

Log:
  MFC r345839, r345894, r345897, r345900-r345901, r345976, r346220, r348602, r348767, r348967,
  r349047-r349048, r351204-r351205, r351502, r351623, r352364, r356249-r356250, r356313,
  r356434, r356628-r356629, r356636, r356657, r357421, r357716, r360359, r360428:
  
  r345839: Assert that q can't be NULL.  'empty' is always non-NULL when DIREMPTY
  r345894: Restore the ability of checking and fixing next free
  r345897: Restore lfcl when LOSTDIR's chain was corrupted and overwritten
  r345900: Implement checking of `.' and `..' entries of subdirectory.
  r345901: Fix build.
  r345976: Write string constant differently to improve readability.
  r346220: Don't cast result from malloc().
  r348602: Don't increment cl after increment.
  r348767: preen should work independently with alwaysyes and alwaysno.
  r348967: Avoid out of boundary access when checking invalid long filenames.
  r349047: Blankspace.  No actual code change.
  r349048: In ask(): override default option if any of alwaysyes/alwaysno/rdonly is
  r351204: Remove redundant check and wrong fix: fat.c checks already take care
  r351205: Use calloc().
  r351502: Comment boot block checks and perform additional sanity checks:
  r351623: Remove unneeded blank line.  No functional change.
  r352364: Avoid mixing cluster numbers and sector numbers. Makes code more readable.
  r356249: Reduce memory footprint of fsck_msdosfs.
  r356250: Revert r356249 for now as it broke GCC builds.
  r356313: Reduce memory footprint of fsck_msdosfs.
  r356434: fsck_msdosfs.8: document -M.
  r356628: Require FAT to occupy at least one sector.
  r356629: Apply typo fix from NetBSD, we have already applied all NetBSD changes so
  r356636: Correct off-by-two issue when determining FAT type.
  r356657: Tighten FAT checks and fix off-by-one error in corner case.
  r357421: Diff reduction against NetBSD, no functional change.
  r357716: Use humanize_number to format available and bad space sizes.
  r360359: Fix a bug with dirty file system handling.
  r360428: Do not overflow when calculating file system size.

Modified:
  stable/11/sbin/fsck_msdosfs/Makefile
  stable/11/sbin/fsck_msdosfs/boot.c
  stable/11/sbin/fsck_msdosfs/check.c
  stable/11/sbin/fsck_msdosfs/dir.c
  stable/11/sbin/fsck_msdosfs/dosfs.h
  stable/11/sbin/fsck_msdosfs/ext.h
  stable/11/sbin/fsck_msdosfs/fat.c
  stable/11/sbin/fsck_msdosfs/fsck_msdosfs.8
  stable/11/sbin/fsck_msdosfs/main.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sbin/fsck_msdosfs/Makefile
==============================================================================
--- stable/11/sbin/fsck_msdosfs/Makefile	Thu Apr 30 05:28:48 2020	(r360489)
+++ stable/11/sbin/fsck_msdosfs/Makefile	Thu Apr 30 06:34:34 2020	(r360490)
@@ -9,6 +9,7 @@ PROG=	fsck_msdosfs
 MAN=	fsck_msdosfs.8
 SRCS=	main.c check.c boot.c fat.c dir.c fsutil.c
 
-CFLAGS+= -I${FSCK}
+CFLAGS+= -I${FSCK} -DHAVE_LIBUTIL_H
+LIBADD=	util
 
 .include <bsd.prog.mk>

Modified: stable/11/sbin/fsck_msdosfs/boot.c
==============================================================================
--- stable/11/sbin/fsck_msdosfs/boot.c	Thu Apr 30 05:28:48 2020	(r360489)
+++ stable/11/sbin/fsck_msdosfs/boot.c	Thu Apr 30 06:34:34 2020	(r360490)
@@ -28,11 +28,14 @@
 
 #include <sys/cdefs.h>
 #ifndef lint
-__RCSID("$NetBSD: boot.c,v 1.11 2006/06/05 16:51:18 christos Exp ");
+__RCSID("$NetBSD: boot.c,v 1.22 2020/01/11 16:29:07 christos Exp $");
 static const char rcsid[] =
   "$FreeBSD$";
 #endif /* not lint */
 
+#include <sys/param.h>
+
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
@@ -46,10 +49,8 @@ readboot(int dosfs, struct bootblock *boot)
 {
 	u_char block[DOSBOOTBLOCKSIZE];
 	u_char fsinfo[2 * DOSBOOTBLOCKSIZE];
-	u_char backup[DOSBOOTBLOCKSIZE];
 	int ret = FSOK;
-	int i;
-	
+
 	if ((size_t)read(dosfs, block, sizeof block) != sizeof block) {
 		perr("could not read boot block");
 		return FSFATAL;
@@ -64,61 +65,130 @@ readboot(int dosfs, struct bootblock *boot)
 	memset(boot, 0, sizeof *boot);
 	boot->ValidFat = -1;
 
-	/* decode bios parameter block */
+	/* Decode BIOS Parameter Block */
+
+	/* Bytes per sector: can only be  512, 1024, 2048 and 4096. */
 	boot->bpbBytesPerSec = block[11] + (block[12] << 8);
+	if (boot->bpbBytesPerSec < DOSBOOTBLOCKSIZE_REAL ||
+	    boot->bpbBytesPerSec > DOSBOOTBLOCKSIZE ||
+	    !powerof2(boot->bpbBytesPerSec)) {
+		pfatal("Invalid sector size: %u", boot->bpbBytesPerSec);
+		return FSFATAL;
+	}
+
+	/* Sectors per cluster: can only be: 1, 2, 4, 8, 16, 32, 64, 128. */
 	boot->bpbSecPerClust = block[13];
+	if (boot->bpbSecPerClust == 0 || !powerof2(boot->bpbSecPerClust)) {
+		pfatal("Invalid cluster size: %u", boot->bpbSecPerClust);
+		return FSFATAL;
+	}
+
+	/* Reserved sectors: must be non-zero */
 	boot->bpbResSectors = block[14] + (block[15] << 8);
+	if (boot->bpbResSectors < 1) {
+		pfatal("Invalid reserved sectors: %u",
+		    boot->bpbResSectors);
+		return FSFATAL;
+	}
+
+	/* Number of FATs */
 	boot->bpbFATs = block[16];
+	if (boot->bpbFATs == 0) {
+		pfatal("Invalid number of FATs: %u", boot->bpbFATs);
+		return FSFATAL;
+	}
+
+	/* Root directory entries for FAT12 and FAT16 */
 	boot->bpbRootDirEnts = block[17] + (block[18] << 8);
+	if (!boot->bpbRootDirEnts) {
+		/* bpbRootDirEnts = 0 suggests that we are FAT32 */
+		boot->flags |= FAT32;
+	}
+
+	/* Total sectors (16 bits) */
 	boot->bpbSectors = block[19] + (block[20] << 8);
+	if (boot->bpbSectors != 0 && (boot->flags & FAT32)) {
+		pfatal("Invalid 16-bit total sector count on FAT32: %u",
+		    boot->bpbSectors);
+		return FSFATAL;
+	}
+
+	/* Media type: ignored */
 	boot->bpbMedia = block[21];
+
+	/* FAT12/FAT16: 16-bit count of sectors per FAT */
 	boot->bpbFATsmall = block[22] + (block[23] << 8);
+	if (boot->bpbFATsmall != 0 && (boot->flags & FAT32)) {
+		pfatal("Invalid 16-bit FAT sector count on FAT32: %u",
+		    boot->bpbFATsmall);
+		return FSFATAL;
+	}
+
+	/* Legacy CHS geometry numbers: ignored */
 	boot->SecPerTrack = block[24] + (block[25] << 8);
 	boot->bpbHeads = block[26] + (block[27] << 8);
+
+	/* Hidden sectors: ignored */
 	boot->bpbHiddenSecs = block[28] + (block[29] << 8) +
 	    (block[30] << 16) + (block[31] << 24);
+
+	/* Total sectors (32 bits) */
 	boot->bpbHugeSectors = block[32] + (block[33] << 8) +
 	    (block[34] << 16) + (block[35] << 24);
-
-	boot->FATsecs = boot->bpbFATsmall;
-
-	if (boot->bpbBytesPerSec % DOSBOOTBLOCKSIZE_REAL != 0 ||
-	    boot->bpbBytesPerSec / DOSBOOTBLOCKSIZE_REAL == 0) {
-		pfatal("Invalid sector size: %u", boot->bpbBytesPerSec);
-		return FSFATAL;
+	if (boot->bpbHugeSectors == 0) {
+		if (boot->flags & FAT32) {
+			pfatal("FAT32 with sector count of zero");
+			return FSFATAL;
+		} else if (boot->bpbSectors == 0) {
+			pfatal("FAT with sector count of zero");
+			return FSFATAL;
+		}
+		boot->NumSectors = boot->bpbSectors;
+	} else {
+		if (boot->bpbSectors != 0) {
+			pfatal("Invalid FAT sector count");
+			return FSFATAL;
+		}
+		boot->NumSectors = boot->bpbHugeSectors;
 	}
-	if (boot->bpbFATs == 0) {
-		pfatal("Invalid number of FATs: %u", boot->bpbFATs);
-		return FSFATAL;
-	}
-	if (!boot->bpbRootDirEnts)
-		boot->flags |= FAT32;
+
 	if (boot->flags & FAT32) {
+		/* If the OEM Name field is EXFAT, it's not FAT32, so bail */
+		if (!memcmp(&block[3], "EXFAT   ", 8)) {
+			pfatal("exFAT filesystem is not supported.");
+			return FSFATAL;
+		}
+
+		/* 32-bit count of sectors per FAT */
 		boot->FATsecs = block[36] + (block[37] << 8)
 				+ (block[38] << 16) + (block[39] << 24);
+
 		if (block[40] & 0x80)
 			boot->ValidFat = block[40] & 0x0f;
 
-		/* check version number: */
+		/* FAT32 version, bail out if not 0.0 */
 		if (block[42] || block[43]) {
-			/* Correct?				XXX */
 			pfatal("Unknown file system version: %x.%x",
 			       block[43], block[42]);
 			return FSFATAL;
 		}
+
+		/*
+		 * Cluster number of the first cluster of root directory.
+		 *
+		 * Should be 2 but do not require it.
+		 */
 		boot->bpbRootClust = block[44] + (block[45] << 8)
 			       + (block[46] << 16) + (block[47] << 24);
+
+		/* Sector number of the FSInfo structure, usually 1 */
 		boot->bpbFSInfo = block[48] + (block[49] << 8);
+
+		/* Sector number of the backup boot block, ignored */
 		boot->bpbBackup = block[50] + (block[51] << 8);
 
-		/* If the OEM Name field is EXFAT, it's not FAT32, so bail */
-		if (!memcmp(&block[3], "EXFAT   ", 8)) {
-			pfatal("exFAT filesystem is not supported.");
-			return FSFATAL;
-		}
-
-		/* check basic parameters */
-		if ((boot->bpbFSInfo == 0) || (boot->bpbSecPerClust == 0)) {
+		/* Check basic parameters */
+		if (boot->bpbFSInfo == 0) {
 			/*
 			 * Either the BIOS Parameter Block has been corrupted,
 			 * or this is not a FAT32 filesystem, most likely an
@@ -127,6 +197,8 @@ readboot(int dosfs, struct bootblock *boot)
 			pfatal("Invalid FAT32 Extended BIOS Parameter Block");
 			return FSFATAL;
 		}
+
+		/* Read in and verify the FSInfo block */
 		if (lseek(dosfs, boot->bpbFSInfo * boot->bpbBytesPerSec,
 		    SEEK_SET) != boot->bpbFSInfo * boot->bpbBytesPerSec
 		    || read(dosfs, fsinfo, sizeof fsinfo) != sizeof fsinfo) {
@@ -164,8 +236,8 @@ readboot(int dosfs, struct bootblock *boot)
 				ret = FSBOOTMOD;
 			} else
 				boot->bpbFSInfo = 0;
-		}
-		if (boot->bpbFSInfo) {
+		} else {
+			/* We appear to have a valid FSInfo block, decode */
 			boot->FSFree = fsinfo[0x1e8] + (fsinfo[0x1e9] << 8)
 				       + (fsinfo[0x1ea] << 16)
 				       + (fsinfo[0x1eb] << 24);
@@ -173,59 +245,58 @@ readboot(int dosfs, struct bootblock *boot)
 				       + (fsinfo[0x1ee] << 16)
 				       + (fsinfo[0x1ef] << 24);
 		}
-
-		if (lseek(dosfs, boot->bpbBackup * boot->bpbBytesPerSec,
-		    SEEK_SET)
-		    != boot->bpbBackup * boot->bpbBytesPerSec
-		    || read(dosfs, backup, sizeof backup) != sizeof  backup) {
-			perr("could not read backup bootblock");
-			return FSFATAL;
-		}
-		backup[65] = block[65];				/* XXX */
-		if (memcmp(block + 11, backup + 11, 79)) {
-			/*
-			 * XXX We require a reference that explains
-			 * that these bytes need to match, or should
-			 * drop the check.  gdt@NetBSD has observed
-			 * filesystems that work fine under Windows XP
-			 * and NetBSD that do not match, so the
-			 * requirement is suspect.  For now, just
-			 * print out useful information and continue.
-			 */
-			pwarn("backup (block %d) mismatch with primary bootblock:\n",
-			        boot->bpbBackup);
-			for (i = 11; i < 11 + 90; i++) {
-				if (block[i] != backup[i])
-					pwarn("\ti=%d\tprimary 0x%02x\tbackup 0x%02x\n",
-					       i, block[i], backup[i]);
-			}
-		}
-		/* Check backup bpbFSInfo?					XXX */
+	} else {
+		/* !FAT32: FAT12/FAT16 */
+		boot->FATsecs = boot->bpbFATsmall;
 	}
 
-	if (boot->bpbSecPerClust == 0) {
-		pfatal("Invalid cluster size: %u", boot->bpbSecPerClust);
+	if (boot->FATsecs < 1 || boot->FATsecs > UINT32_MAX / boot->bpbFATs) {
+		pfatal("Invalid FATs(%u) with FATsecs(%zu)",
+			boot->bpbFATs, (size_t)boot->FATsecs);
 		return FSFATAL;
 	}
-	if (boot->bpbSectors) {
-		boot->bpbHugeSectors = 0;
-		boot->NumSectors = boot->bpbSectors;
-	} else
-		boot->NumSectors = boot->bpbHugeSectors;
-	boot->ClusterOffset = (boot->bpbRootDirEnts * 32 +
+
+	boot->FirstCluster = (boot->bpbRootDirEnts * 32 +
 	    boot->bpbBytesPerSec - 1) / boot->bpbBytesPerSec +
-	    boot->bpbResSectors + boot->bpbFATs * boot->FATsecs -
-	    CLUST_FIRST * boot->bpbSecPerClust;
-	boot->NumClusters = (boot->NumSectors - boot->ClusterOffset) /
-	    boot->bpbSecPerClust;
+	    boot->bpbResSectors + boot->bpbFATs * boot->FATsecs;
 
-	if (boot->flags&FAT32)
+	if (boot->FirstCluster + boot->bpbSecPerClust > boot->NumSectors) {
+		pfatal("Cluster offset too large (%u clusters)\n",
+		    boot->FirstCluster);
+		return FSFATAL;
+	}
+
+	/*
+	 * 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)) {
+			pfatal("Filesystem too big (%u clusters) for FAT32 partition",
+			    boot->NumClusters);
+			return FSFATAL;
+		}
+		if (boot->NumClusters < (CLUST_RSRVD & CLUST16_MASK)) {
+			pfatal("Filesystem too small (%u clusters) for FAT32 partition",
+			    boot->NumClusters);
+			return FSFATAL;
+		}
 		boot->ClustMask = CLUST32_MASK;
-	else if (boot->NumClusters < (CLUST_RSRVD&CLUST12_MASK))
+
+		if (boot->bpbRootClust < CLUST_FIRST ||
+		    boot->bpbRootClust >= boot->NumClusters) {
+			pfatal("Root directory starts with cluster out of range(%u)",
+			       boot->bpbRootClust);
+			return FSFATAL;
+		}
+	} else if (boot->NumClusters < (CLUST_RSRVD&CLUST12_MASK)) {
 		boot->ClustMask = CLUST12_MASK;
-	else if (boot->NumClusters < (CLUST_RSRVD&CLUST16_MASK))
+	} else if (boot->NumClusters < (CLUST_RSRVD&CLUST16_MASK)) {
 		boot->ClustMask = CLUST16_MASK;
-	else {
+	} else {
 		pfatal("Filesystem too big (%u clusters) for non-FAT32 partition",
 		       boot->NumClusters);
 		return FSFATAL;
@@ -243,11 +314,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 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;
@@ -289,7 +368,7 @@ writefsinfo(int dosfs, struct bootblock *boot)
 	 * support for FAT32) doesn't maintain the FSINFO block
 	 * correctly, it has to be fixed pretty often.
 	 *
-	 * Therefor, we handle the FSINFO block only informally,
+	 * Therefore, we handle the FSINFO block only informally,
 	 * fixing it if necessary, but otherwise ignoring the
 	 * fact that it was incorrect.
 	 */

Modified: stable/11/sbin/fsck_msdosfs/check.c
==============================================================================
--- stable/11/sbin/fsck_msdosfs/check.c	Thu Apr 30 05:28:48 2020	(r360489)
+++ stable/11/sbin/fsck_msdosfs/check.c	Thu Apr 30 06:34:34 2020	(r360490)
@@ -33,6 +33,9 @@ static const char rcsid[] =
   "$FreeBSD$";
 #endif /* not lint */
 
+#ifdef HAVE_LIBUTIL_H
+#include <libutil.h>
+#endif
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
@@ -47,11 +50,12 @@ checkfilesys(const char *fname)
 {
 	int dosfs;
 	struct bootblock boot;
-	struct fatEntry *fat = NULL;
+	struct fat_descriptor *fat = NULL;
 	int finish_dosdirsection=0;
-	u_int i;
 	int mod = 0;
 	int ret = 8;
+	int64_t freebytes;
+	int64_t badbytes;
 
 	rdonly = alwaysno;
 	if (!preen)
@@ -88,80 +92,73 @@ checkfilesys(const char *fname)
 	}
 
 	if (!preen)  {
-		if (boot.ValidFat < 0)
-			printf("** Phase 1 - Read and Compare FATs\n");
-		else
-			printf("** Phase 1 - Read FAT\n");
+		printf("** Phase 1 - Read FAT and checking connectivity\n");
 	}
 
-	mod |= readfat(dosfs, &boot, boot.ValidFat >= 0 ? boot.ValidFat : 0, &fat);
+	mod |= readfat(dosfs, &boot, &fat);
 	if (mod & FSFATAL) {
 		close(dosfs);
 		return 8;
 	}
 
-	if (boot.ValidFat < 0)
-		for (i = 1; i < boot.bpbFATs; i++) {
-			struct fatEntry *currentFat;
-
-			mod |= readfat(dosfs, &boot, i, &currentFat);
-
-			if (mod & FSFATAL)
-				goto out;
-
-			mod |= comparefat(&boot, fat, currentFat, i);
-			free(currentFat);
-			if (mod & FSFATAL)
-				goto out;
-		}
-
 	if (!preen)
-		printf("** Phase 2 - Check Cluster Chains\n");
+		printf("** Phase 2 - Checking Directories\n");
 
-	mod |= checkfat(&boot, fat);
-	if (mod & FSFATAL)
-		goto out;
-	/* delay writing FATs */
-
-	if (!preen)
-		printf("** Phase 3 - Checking Directories\n");
-
-	mod |= resetDosDirSection(&boot, fat);
+	mod |= resetDosDirSection(fat);
 	finish_dosdirsection = 1;
 	if (mod & FSFATAL)
 		goto out;
 	/* delay writing FATs */
 
-	mod |= handleDirTree(dosfs, &boot, fat);
+	mod |= handleDirTree(fat);
 	if (mod & FSFATAL)
 		goto out;
 
 	if (!preen)
-		printf("** Phase 4 - Checking for Lost Files\n");
+		printf("** Phase 3 - Checking for Lost Files\n");
 
-	mod |= checklost(dosfs, &boot, fat);
+	mod |= checklost(fat);
 	if (mod & FSFATAL)
 		goto out;
 
 	/* now write the FATs */
-	if (mod & (FSFATMOD|FSFIXFAT)) {
+	if (mod & FSFATMOD) {
 		if (ask(1, "Update FATs")) {
-			mod |= writefat(dosfs, &boot, fat, mod & FSFIXFAT);
+			mod |= writefat(fat);
 			if (mod & FSFATAL)
 				goto out;
 		} else
 			mod |= FSERROR;
 	}
 
+	freebytes = (int64_t)boot.NumFree * boot.ClusterSize;
+	badbytes = (int64_t)boot.NumBad * boot.ClusterSize;
+
+#ifdef HAVE_LIBUTIL_H
+	char freestr[7], badstr[7];
+
+	humanize_number(freestr, sizeof(freestr), freebytes, "",
+	    HN_AUTOSCALE, HN_DECIMAL | HN_IEC_PREFIXES);
+	if (boot.NumBad) {
+		humanize_number(badstr, sizeof(badstr), badbytes, "",
+		    HN_AUTOSCALE, HN_B | HN_DECIMAL | HN_IEC_PREFIXES);
+
+		pwarn("%d files, %sB free (%d clusters), %sB bad (%d clusters)\n",
+		      boot.NumFiles, freestr, boot.NumFree,
+		      badstr, boot.NumBad);
+	} else {
+		pwarn("%d files, %sB free (%d clusters)\n",
+		      boot.NumFiles, freestr, boot.NumFree);
+	}
+#else
 	if (boot.NumBad)
-		pwarn("%d files, %d free (%d clusters), %d bad (%d clusters)\n",
-		      boot.NumFiles,
-		      boot.NumFree * boot.ClusterSize / 1024, boot.NumFree,
-		      boot.NumBad * boot.ClusterSize / 1024, boot.NumBad);
+		pwarn("%d files, %jd KiB free (%d clusters), %jd KiB bad (%d clusters)\n",
+		      boot.NumFiles, (intmax_t)freebytes / 1024, boot.NumFree,
+		      (intmax_t)badbytes / 1024, boot.NumBad);
 	else
-		pwarn("%d files, %d free (%d clusters)\n",
-		      boot.NumFiles,
-		      boot.NumFree * boot.ClusterSize / 1024, boot.NumFree);
+		pwarn("%d files, %jd KiB free (%d clusters)\n",
+		      boot.NumFiles, (intmax_t)freebytes / 1024, boot.NumFree);
+#endif
 
 	if (mod && (mod & FSERROR) == 0) {
 		if (mod & FSDIRTY) {
@@ -170,7 +167,7 @@ checkfilesys(const char *fname)
 
 			if (mod & FSDIRTY) {
 				pwarn("MARKING FILE SYSTEM CLEAN\n");
-				mod |= writefat(dosfs, &boot, fat, 1);
+				mod |= cleardirty(fat);
 			} else {
 				pwarn("\n***** FILE SYSTEM IS LEFT MARKED AS DIRTY *****\n");
 				mod |= FSERROR; /* file system not clean */

Modified: stable/11/sbin/fsck_msdosfs/dir.c
==============================================================================
--- stable/11/sbin/fsck_msdosfs/dir.c	Thu Apr 30 05:28:48 2020	(r360489)
+++ stable/11/sbin/fsck_msdosfs/dir.c	Thu Apr 30 06:34:34 2020	(r360490)
@@ -1,6 +1,7 @@
 /*-
  * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
  *
+ * Copyright (c) 2019 Google LLC
  * Copyright (C) 1995, 1996, 1997 Wolfgang Solfrank
  * Copyright (c) 1995 Martin Husemann
  * Some structure declaration borrowed from Paul Popelka
@@ -35,6 +36,7 @@ static const char rcsid[] =
   "$FreeBSD$";
 #endif /* not lint */
 
+#include <assert.h>
 #include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -94,14 +96,11 @@ static struct dirTodoNode *newDirTodo(void);
 static void freeDirTodo(struct dirTodoNode *);
 static char *fullpath(struct dosDirEntry *);
 static u_char calcShortSum(u_char *);
-static int delete(int, struct bootblock *, struct fatEntry *, cl_t, int,
-    cl_t, int, int);
-static int removede(int, struct bootblock *, struct fatEntry *, u_char *,
-    u_char *, cl_t, cl_t, cl_t, char *, int);
-static int checksize(struct bootblock *, struct fatEntry *, u_char *,
-    struct dosDirEntry *);
-static int readDosDirSection(int, struct bootblock *, struct fatEntry *,
-    struct dosDirEntry *);
+static int delete(struct fat_descriptor *, cl_t, int, cl_t, int, int);
+static int removede(struct fat_descriptor *, u_char *, u_char *,
+    cl_t, cl_t, cl_t, char *, int);
+static int checksize(struct fat_descriptor *, u_char *, struct dosDirEntry *);
+static int readDosDirSection(struct fat_descriptor *, struct dosDirEntry *);
 
 /*
  * Manage free dosDirEntry structures.
@@ -114,8 +113,8 @@ newDosDirEntry(void)
 	struct dosDirEntry *de;
 
 	if (!(de = freede)) {
-		if (!(de = (struct dosDirEntry *)malloc(sizeof *de)))
-			return 0;
+		if (!(de = malloc(sizeof *de)))
+			return (NULL);
 	} else
 		freede = de->next;
 	return de;
@@ -139,7 +138,7 @@ newDirTodo(void)
 	struct dirTodoNode *dt;
 
 	if (!(dt = freedt)) {
-		if (!(dt = (struct dirTodoNode *)malloc(sizeof *dt)))
+		if (!(dt = malloc(sizeof *dt)))
 			return 0;
 	} else
 		freedt = dt->next;
@@ -192,7 +191,7 @@ fullpath(struct dosDirEntry *dir)
 /*
  * Calculate a checksum over an 8.3 alias name
  */
-static u_char
+static inline u_char
 calcShortSum(u_char *p)
 {
 	u_char sum = 0;
@@ -220,22 +219,24 @@ static struct dosDirEntry *lostDir;
  * Init internal state for a new directory scan.
  */
 int
-resetDosDirSection(struct bootblock *boot, struct fatEntry *fat)
+resetDosDirSection(struct fat_descriptor *fat)
 {
-	int b1, b2;
-	cl_t cl;
+	int rootdir_size, cluster_size;
 	int ret = FSOK;
 	size_t len;
+	struct bootblock *boot;
 
-	b1 = boot->bpbRootDirEnts * 32;
-	b2 = boot->bpbSecPerClust * boot->bpbBytesPerSec;
+	boot = fat_get_boot(fat);
 
-	if ((buffer = malloc(len = MAX(b1, b2))) == NULL) {
+	rootdir_size = boot->bpbRootDirEnts * 32;
+	cluster_size = boot->bpbSecPerClust * boot->bpbBytesPerSec;
+
+	if ((buffer = malloc(len = MAX(rootdir_size, cluster_size))) == NULL) {
 		perr("No space for directory buffer (%zu)", len);
 		return FSFATAL;
 	}
 
-	if ((delbuf = malloc(len = b2)) == NULL) {
+	if ((delbuf = malloc(len = cluster_size)) == NULL) {
 		free(buffer);
 		perr("No space for directory delbuf (%zu)", len);
 		return FSFATAL;
@@ -250,33 +251,10 @@ resetDosDirSection(struct bootblock *boot, struct fatE
 
 	memset(rootDir, 0, sizeof *rootDir);
 	if (boot->flags & FAT32) {
-		if (boot->bpbRootClust < CLUST_FIRST ||
-		    boot->bpbRootClust >= boot->NumClusters) {
-			pfatal("Root directory starts with cluster out of range(%u)",
-			       boot->bpbRootClust);
+		if (!fat_is_cl_head(fat, boot->bpbRootClust)) {
+			pfatal("Root directory doesn't start a cluster chain");
 			return FSFATAL;
 		}
-		cl = fat[boot->bpbRootClust].next;
-		if (cl < CLUST_FIRST
-		    || (cl >= CLUST_RSRVD && cl< CLUST_EOFS)
-		    || fat[boot->bpbRootClust].head != boot->bpbRootClust) {
-			if (cl == CLUST_FREE)
-				pwarn("Root directory starts with free cluster\n");
-			else if (cl >= CLUST_RSRVD)
-				pwarn("Root directory starts with cluster marked %s\n",
-				      rsrvdcltype(cl));
-			else {
-				pfatal("Root directory doesn't start a cluster chain");
-				return FSFATAL;
-			}
-			if (ask(1, "Fix")) {
-				fat[boot->bpbRootClust].next = CLUST_FREE;
-				ret = FSFATMOD;
-			} else
-				ret = FSFATAL;
-		}
-
-		fat[boot->bpbRootClust].flags |= FAT_USED;
 		rootDir->head = boot->bpbRootClust;
 	}
 
@@ -317,28 +295,34 @@ finishDosDirSection(void)
  * Delete directory entries between startcl, startoff and endcl, endoff.
  */
 static int
-delete(int f, struct bootblock *boot, struct fatEntry *fat, cl_t startcl,
+delete(struct fat_descriptor *fat, cl_t startcl,
     int startoff, cl_t endcl, int endoff, int notlast)
 {
 	u_char *s, *e;
 	off_t off;
-	int clsz = boot->bpbSecPerClust * boot->bpbBytesPerSec;
+	int clsz, fd;
+	struct bootblock *boot;
 
+	boot = fat_get_boot(fat);
+	fd = fat_get_fd(fat);
+	clsz = boot->bpbSecPerClust * boot->bpbBytesPerSec;
+
 	s = delbuf + startoff;
 	e = delbuf + clsz;
-	while (startcl >= CLUST_FIRST && startcl < boot->NumClusters) {
+	while (fat_is_valid_cl(fat, startcl)) {
 		if (startcl == endcl) {
 			if (notlast)
 				break;
 			e = delbuf + endoff;
 		}
-		off = startcl * boot->bpbSecPerClust + boot->ClusterOffset;
+		off = (startcl - CLUST_FIRST) * boot->bpbSecPerClust + boot->FirstCluster;
+
 		off *= boot->bpbBytesPerSec;
-		if (lseek(f, off, SEEK_SET) != off) {
+		if (lseek(fd, off, SEEK_SET) != off) {
 			perr("Unable to lseek to %" PRId64, off);
 			return FSFATAL;
 		}
-		if (read(f, delbuf, clsz) != clsz) {
+		if (read(fd, delbuf, clsz) != clsz) {
 			perr("Unable to read directory");
 			return FSFATAL;
 		}
@@ -346,25 +330,26 @@ delete(int f, struct bootblock *boot, struct fatEntry 
 			*s = SLOT_DELETED;
 			s += 32;
 		}
-		if (lseek(f, off, SEEK_SET) != off) {
+		if (lseek(fd, off, SEEK_SET) != off) {
 			perr("Unable to lseek to %" PRId64, off);
 			return FSFATAL;
 		}
-		if (write(f, delbuf, clsz) != clsz) {
+		if (write(fd, delbuf, clsz) != clsz) {
 			perr("Unable to write directory");
 			return FSFATAL;
 		}
 		if (startcl == endcl)
 			break;
-		startcl = fat[startcl].next;
+		startcl = fat_get_cl_next(fat, startcl);
 		s = delbuf;
 	}
 	return FSOK;
 }
 
 static int
-removede(int f, struct bootblock *boot, struct fatEntry *fat, u_char *start,
-    u_char *end, cl_t startcl, cl_t endcl, cl_t curcl, char *path, int type)
+removede(struct fat_descriptor *fat, u_char *start,
+    u_char *end, cl_t startcl, cl_t endcl, cl_t curcl,
+    char *path, int type)
 {
 	switch (type) {
 	case 0:
@@ -380,14 +365,14 @@ removede(int f, struct bootblock *boot, struct fatEntr
 	}
 	if (ask(0, "Remove")) {
 		if (startcl != curcl) {
-			if (delete(f, boot, fat,
+			if (delete(fat,
 				   startcl, start - buffer,
 				   endcl, end - buffer,
 				   endcl == curcl) == FSFATAL)
 				return FSFATAL;
 			start = buffer;
 		}
-		/* startcl is < CLUST_FIRST for !fat32 root */
+		/* startcl is < CLUST_FIRST for !FAT32 root */
 		if ((endcl == curcl) || (startcl < CLUST_FIRST))
 			for (; start < end; start += 32)
 				*start = SLOT_DELETED;
@@ -400,23 +385,37 @@ removede(int f, struct bootblock *boot, struct fatEntr
  * Check an in-memory file entry
  */
 static int
-checksize(struct bootblock *boot, struct fatEntry *fat, u_char *p,
-    struct dosDirEntry *dir)
+checksize(struct fat_descriptor *fat, u_char *p, struct dosDirEntry *dir)
 {
+	int ret = FSOK;
+	size_t physicalSize;
+	struct bootblock *boot;
+
+	boot = fat_get_boot(fat);
+
 	/*
 	 * Check size on ordinary files
 	 */
-	u_int32_t physicalSize;
-
-	if (dir->head == CLUST_FREE)
+	if (dir->head == CLUST_FREE) {
 		physicalSize = 0;
-	else {
-		if (dir->head < CLUST_FIRST || dir->head >= boot->NumClusters)
+	} else {
+		if (!fat_is_valid_cl(fat, dir->head))
 			return FSERROR;
-		physicalSize = fat[dir->head].length * boot->ClusterSize;
+		ret = checkchain(fat, dir->head, &physicalSize);
+		/*
+		 * Upon return, physicalSize 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
+		 * case.
+		 */
+		if (ret == FSERROR) {
+			return (FSERROR);
+		}
+		physicalSize *= boot->ClusterSize;
 	}
 	if (physicalSize < dir->size) {
-		pwarn("size of %s is %u, should at most be %u\n",
+		pwarn("size of %s is %u, should at most be %zu\n",
 		      fullpath(dir), dir->size, physicalSize);
 		if (ask(1, "Truncate")) {
 			dir->size = physicalSize;
@@ -436,40 +435,118 @@ checksize(struct bootblock *boot, struct fatEntry *fat
 
 			for (cl = dir->head, len = sz = 0;
 			    (sz += boot->ClusterSize) < dir->size; len++)
-				cl = fat[cl].next;
-			clearchain(boot, fat, fat[cl].next);
-			fat[cl].next = CLUST_EOF;
-			fat[dir->head].length = len;
-			return FSFATMOD;
+				cl = fat_get_cl_next(fat, cl);
+			clearchain(fat, fat_get_cl_next(fat, cl));
+			ret = fat_set_cl_next(fat, cl, CLUST_EOF);
+			return (FSFATMOD | ret);
 		} else
 			return FSERROR;
 	}
 	return FSOK;
 }
 
+static const u_char dot_name[11]    = ".          ";
+static const u_char dotdot_name[11] = "..         ";
+
 /*
+ * Basic sanity check if the subdirectory have good '.' and '..' entries,
+ * and they are directory entries.  Further sanity checks are performed
+ * when we traverse into it.
+ */
+static int
+check_subdirectory(struct fat_descriptor *fat, struct dosDirEntry *dir)
+{
+	u_char *buf, *cp;
+	off_t off;
+	cl_t cl;
+	int retval = FSOK;
+	int fd;
+	struct bootblock *boot;
+
+	boot = fat_get_boot(fat);
+	fd = fat_get_fd(fat);
+
+	cl = dir->head;
+	if (dir->parent && !fat_is_valid_cl(fat, cl)) {
+		return FSERROR;
+	}
+
+	if (!(boot->flags & FAT32) && !dir->parent) {
+		off = boot->bpbResSectors + boot->bpbFATs *
+			boot->FATsecs;
+	} else {
+		off = (cl - CLUST_FIRST) * boot->bpbSecPerClust + boot->FirstCluster;
+	}
+
+	/*
+	 * We only need to check the first two entries of the directory,
+	 * which is found in the first sector of the directory entry,
+	 * so read in only the first sector.
+	 */
+	buf = malloc(boot->bpbBytesPerSec);
+	if (buf == NULL) {
+		perr("No space for directory buffer (%u)",
+		    boot->bpbBytesPerSec);
+		return FSFATAL;
+	}
+
+	off *= boot->bpbBytesPerSec;
+	if (lseek(fd, off, SEEK_SET) != off ||
+	    read(fd, buf, boot->bpbBytesPerSec) != (ssize_t)boot->bpbBytesPerSec) {
+		perr("Unable to read directory");
+		free(buf);
+		return FSFATAL;
+	}
+
+	/*
+	 * Both `.' and `..' must be present and be the first two entries
+	 * and be ATTR_DIRECTORY of a valid subdirectory.
+	 */
+	cp = buf;
+	if (memcmp(cp, dot_name, sizeof(dot_name)) != 0 ||
+	    (cp[11] & ATTR_DIRECTORY) != ATTR_DIRECTORY) {
+		pwarn("%s: Incorrect `.' for %s.\n", __func__, dir->name);
+		retval |= FSERROR;
+	}
+	cp += 32;
+	if (memcmp(cp, dotdot_name, sizeof(dotdot_name)) != 0 ||
+	    (cp[11] & ATTR_DIRECTORY) != ATTR_DIRECTORY) {
+		pwarn("%s: Incorrect `..' for %s. \n", __func__, dir->name);
+		retval |= FSERROR;
+	}
+
+	free(buf);
+	return retval;
+}
+
+/*
  * Read a directory and
  *   - resolve long name records
  *   - enter file and directory records into the parent's list
  *   - push directories onto the todo-stack
  */
 static int
-readDosDirSection(int f, struct bootblock *boot, struct fatEntry *fat,
-    struct dosDirEntry *dir)
+readDosDirSection(struct fat_descriptor *fat, struct dosDirEntry *dir)
 {
+	struct bootblock *boot;
 	struct dosDirEntry dirent, *d;
 	u_char *p, *vallfn, *invlfn, *empty;
 	off_t off;
-	int i, j, k, last;
+	int fd, i, j, k, iosize, entries;
+	bool is_legacyroot;
 	cl_t cl, valcl = ~0, invcl = ~0, empcl = ~0;
 	char *t;
 	u_int lidx = 0;
 	int shortSum;
 	int mod = FSOK;
+	size_t dirclusters;
 #define	THISMOD	0x8000			/* Only used within this routine */
 
+	boot = fat_get_boot(fat);
+	fd = fat_get_fd(fat);
+
 	cl = dir->head;
-	if (dir->parent && (cl < CLUST_FIRST || cl >= boot->NumClusters)) {
+	if (dir->parent && (!fat_is_valid_cl(fat, cl))) {
 		/*
 		 * Already handled somewhere else.
 		 */
@@ -477,27 +554,50 @@ readDosDirSection(int f, struct bootblock *boot, struc
 	}
 	shortSum = -1;
 	vallfn = invlfn = empty = NULL;
+
+	/*
+	 * If we are checking the legacy root (for FAT12/FAT16),
+	 * we will operate on the whole directory; otherwise, we
+	 * will operate on one cluster at a time, and also take
+	 * this opportunity to examine the chain.
+	 *
+	 * Derive how many entries we are going to encounter from
+	 * the I/O size.
+	 */
+	is_legacyroot = (dir->parent == NULL && !(boot->flags & FAT32));
+	if (is_legacyroot) {
+		iosize = boot->bpbRootDirEnts * 32;
+		entries = boot->bpbRootDirEnts;
+	} else {
+		iosize = boot->bpbSecPerClust * boot->bpbBytesPerSec;
+		entries = iosize / 32;
+		mod |= checkchain(fat, dir->head, &dirclusters);
+	}
+
 	do {
-		if (!(boot->flags & FAT32) && !dir->parent) {
-			last = boot->bpbRootDirEnts * 32;
+		if (is_legacyroot) {
+			/*
+			 * Special case for FAT12/FAT16 root -- read
+			 * in the whole root directory.
+			 */
 			off = boot->bpbResSectors + boot->bpbFATs *
 			    boot->FATsecs;
 		} else {
-			last = boot->bpbSecPerClust * boot->bpbBytesPerSec;
-			off = cl * boot->bpbSecPerClust + boot->ClusterOffset;
+			/*
+			 * Otherwise, read in a cluster of the
+			 * directory.
+			 */
+			off = (cl - CLUST_FIRST) * boot->bpbSecPerClust + boot->FirstCluster;
 		}
 
 		off *= boot->bpbBytesPerSec;
-		if (lseek(f, off, SEEK_SET) != off
-		    || read(f, buffer, last) != last) {
+		if (lseek(fd, off, SEEK_SET) != off ||
+		    read(fd, buffer, iosize) != iosize) {
 			perr("Unable to read directory");
 			return FSFATAL;
 		}
-		last /= 32;
-		/*
-		 * Check `.' and `..' entries here?			XXX
-		 */
-		for (p = buffer, i = 0; i < last; i++, p += 32) {
+
+		for (p = buffer, i = 0; i < entries; i++, p += 32) {
 			if (dir->fsckflags & DIREMPWARN) {
 				*p = SLOT_EMPTY;
 				continue;
@@ -520,11 +620,12 @@ readDosDirSection(int f, struct bootblock *boot, struc
 						u_char *q;
 
 						dir->fsckflags &= ~DIREMPTY;
-						if (delete(f, boot, fat,
+						if (delete(fat,
 							   empcl, empty - buffer,
 							   cl, p - buffer, 1) == FSFATAL)
 							return FSFATAL;
-						q = empcl == cl ? empty : buffer;
+						q = ((empcl == cl) ? empty : buffer);
+						assert(q != NULL);
 						for (; q < p; q += 32)
 							*q = SLOT_DELETED;
 						mod |= THISMOD|FSDIRMOD;
@@ -565,6 +666,15 @@ readDosDirSection(int f, struct bootblock *boot, struc
 					vallfn = NULL;
 				}
 				lidx = *p & LRNOMASK;
+				if (lidx == 0) {
+					pwarn("invalid long name\n");
+					if (!invlfn) {
+						invlfn = vallfn;
+						invcl = valcl;
+					}
+					vallfn = NULL;
+					continue;
+				}
 				t = longName + --lidx * 13;
 				for (k = 1; k < 11 && t < longName +
 				    sizeof(longName); k += 2) {
@@ -639,7 +749,7 @@ readDosDirSection(int f, struct bootblock *boot, struc
 
 			if (dirent.flags & ATTR_VOLUME) {
 				if (vallfn || invlfn) {
-					mod |= removede(f, boot, fat,
+					mod |= removede(fat,

*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***



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