Date: Sat, 30 Nov 2019 05:43:25 +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-12@freebsd.org Subject: svn commit: r355221 - stable/12/sbin/fsck_msdosfs Message-ID: <201911300543.xAU5hPST044490@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: delphij Date: Sat Nov 30 05:43:24 2019 New Revision: 355221 URL: https://svnweb.freebsd.org/changeset/base/355221 Log: MFC r345839, r345894, r345897, r345900-r345901, r345976, r346220, r348602, r348767, r348967, r349047-r349048, r351502, r351623, r352364 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 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. Modified: stable/12/sbin/fsck_msdosfs/boot.c stable/12/sbin/fsck_msdosfs/dir.c stable/12/sbin/fsck_msdosfs/dosfs.h stable/12/sbin/fsck_msdosfs/fat.c stable/12/sbin/fsck_msdosfs/main.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sbin/fsck_msdosfs/boot.c ============================================================================== --- stable/12/sbin/fsck_msdosfs/boot.c Sat Nov 30 05:01:12 2019 (r355220) +++ stable/12/sbin/fsck_msdosfs/boot.c Sat Nov 30 05:43:24 2019 (r355221) @@ -33,6 +33,9 @@ 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,133 @@ 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); + 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; + } - 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->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 +200,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 +239,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,53 +248,31 @@ 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 > 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; + } + + boot->NumClusters = (boot->NumSectors - boot->FirstCluster) / boot->bpbSecPerClust + + CLUST_FIRST; + + if (boot->flags & FAT32) boot->ClustMask = CLUST32_MASK; else if (boot->NumClusters < (CLUST_RSRVD&CLUST12_MASK)) boot->ClustMask = CLUST12_MASK; Modified: stable/12/sbin/fsck_msdosfs/dir.c ============================================================================== --- stable/12/sbin/fsck_msdosfs/dir.c Sat Nov 30 05:01:12 2019 (r355220) +++ stable/12/sbin/fsck_msdosfs/dir.c Sat Nov 30 05:43:24 2019 (r355221) @@ -35,6 +35,7 @@ static const char rcsid[] = "$FreeBSD$"; #endif /* not lint */ +#include <assert.h> #include <inttypes.h> #include <stdio.h> #include <stdlib.h> @@ -114,7 +115,7 @@ newDosDirEntry(void) struct dosDirEntry *de; if (!(de = freede)) { - if (!(de = (struct dosDirEntry *)malloc(sizeof *de))) + if (!(de = malloc(sizeof *de))) return 0; } else freede = de->next; @@ -139,7 +140,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; @@ -316,7 +317,8 @@ delete(int f, struct bootblock *boot, struct fatEntry 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) { perr("Unable to lseek to %" PRId64, off); @@ -431,7 +433,76 @@ checksize(struct bootblock *boot, struct fatEntry *fat 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(int f, struct bootblock *boot, struct dosDirEntry *dir) +{ + u_char *buf, *cp; + off_t off; + cl_t cl; + int retval = FSOK; + + cl = dir->head; + if (dir->parent && (cl < CLUST_FIRST || cl >= boot->NumClusters)) { + 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(f, off, SEEK_SET) != off || + read(f, 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 @@ -468,7 +539,7 @@ readDosDirSection(int f, struct bootblock *boot, struc boot->FATsecs; } else { last = boot->bpbSecPerClust * boot->bpbBytesPerSec; - off = cl * boot->bpbSecPerClust + boot->ClusterOffset; + off = (cl - CLUST_FIRST) * boot->bpbSecPerClust + boot->FirstCluster; } off *= boot->bpbBytesPerSec; @@ -478,9 +549,6 @@ readDosDirSection(int f, struct bootblock *boot, struc return FSFATAL; } last /= 32; - /* - * Check `.' and `..' entries here? XXX - */ for (p = buffer, i = 0; i < last; i++, p += 32) { if (dir->fsckflags & DIREMPWARN) { *p = SLOT_EMPTY; @@ -508,7 +576,8 @@ readDosDirSection(int f, struct bootblock *boot, struc 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; @@ -549,6 +618,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) { @@ -816,6 +894,36 @@ readDosDirSection(int f, struct bootblock *boot, struc } } continue; + } else { + /* + * Only one directory entry can point + * to dir->head, it's '.'. + */ + if (dirent.head == dir->head) { + pwarn("%s entry in %s has incorrect start cluster\n", + dirent.name, fullpath(dir)); + if (ask(1, "Remove")) { + *p = SLOT_DELETED; + mod |= THISMOD|FSDIRMOD; + } else + mod |= FSERROR; + continue; + } else if ((check_subdirectory(f, boot, + &dirent) & FSERROR) == FSERROR) { + /* + * A subdirectory should have + * a dot (.) entry and a dot-dot + * (..) entry of ATTR_DIRECTORY, + * we will inspect further when + * traversing into it. + */ + if (ask(1, "Remove")) { + *p = SLOT_DELETED; + mod |= THISMOD|FSDIRMOD; + } else + mod |= FSERROR; + continue; + } } /* create directory tree node */ @@ -959,10 +1067,12 @@ reconnect(int dosfs, struct bootblock *boot, struct fa if (lfcl < CLUST_FIRST || lfcl >= boot->NumClusters) { /* Extend LOSTDIR? XXX */ pwarn("No space in %s\n", LOSTDIR); + lfcl = (lostDir->head < boot->NumClusters) ? lostDir->head : 0; return FSERROR; } - lfoff = lfcl * boot->ClusterSize - + boot->ClusterOffset * boot->bpbBytesPerSec; + lfoff = (lfcl - CLUST_FIRST) * boot->ClusterSize + + boot->FirstCluster * boot->bpbBytesPerSec; + if (lseek(dosfs, lfoff, SEEK_SET) != lfoff || (size_t)read(dosfs, lfbuf, boot->ClusterSize) != boot->ClusterSize) { perr("could not read LOST.DIR"); Modified: stable/12/sbin/fsck_msdosfs/dosfs.h ============================================================================== --- stable/12/sbin/fsck_msdosfs/dosfs.h Sat Nov 30 05:01:12 2019 (r355220) +++ stable/12/sbin/fsck_msdosfs/dosfs.h Sat Nov 30 05:43:24 2019 (r355221) @@ -74,7 +74,7 @@ struct bootblock { u_int32_t NumSectors; /* how many sectors are there */ u_int32_t FATsecs; /* how many sectors are in FAT */ u_int32_t NumFatEntries; /* how many entries really are there */ - u_int ClusterOffset; /* at what sector would sector 0 start */ + u_int FirstCluster; /* at what sector is Cluster CLUST_FIRST */ u_int ClusterSize; /* Cluster size in bytes */ /* Now some statistics: */ Modified: stable/12/sbin/fsck_msdosfs/fat.c ============================================================================== --- stable/12/sbin/fsck_msdosfs/fat.c Sat Nov 30 05:01:12 2019 (r355220) +++ stable/12/sbin/fsck_msdosfs/fat.c Sat Nov 30 05:43:24 2019 (r355221) @@ -518,7 +518,6 @@ clear: } if (head == fat[n].head) { pwarn("Cluster chain starting at %u loops at cluster %u\n", - head, p); goto clear; } @@ -645,8 +644,8 @@ writefat(int fs, struct bootblock *boot, struct fatEnt break; if (fat[cl].next == CLUST_FREE) boot->NumFree++; - *p++ |= (u_char)(fat[cl + 1].next << 4); - *p++ = (u_char)(fat[cl + 1].next >> 4); + *p++ |= (u_char)(fat[cl].next << 4); + *p++ = (u_char)(fat[cl].next >> 4); break; } } @@ -704,6 +703,20 @@ checklost(int dosfs, struct bootblock *boot, struct fa boot->FSFree = boot->NumFree; ret = 1; } + } + if (boot->FSNext != 0xffffffffU && + (boot->FSNext >= boot->NumClusters || + (boot->NumFree && fat[boot->FSNext].next != CLUST_FREE))) { + pwarn("Next free cluster in FSInfo block (%u) %s\n", + boot->FSNext, + (boot->FSNext >= boot->NumClusters) ? "invalid" : "not free"); + if (ask(1, "fix")) + for (head = CLUST_FIRST; head < boot->NumClusters; head++) + if (fat[head].next == CLUST_FREE) { + boot->FSNext = head; + ret = 1; + break; + } } if (ret) mod |= writefsinfo(dosfs, boot); Modified: stable/12/sbin/fsck_msdosfs/main.c ============================================================================== --- stable/12/sbin/fsck_msdosfs/main.c Sat Nov 30 05:01:12 2019 (r355220) +++ stable/12/sbin/fsck_msdosfs/main.c Sat Nov 30 05:43:24 2019 (r355221) @@ -87,16 +87,15 @@ main(int argc, char **argv) exit(5); case 'n': alwaysno = 1; - alwaysyes = preen = 0; + alwaysyes = 0; break; case 'y': alwaysyes = 1; - alwaysno = preen = 0; + alwaysno = 0; break; case 'p': preen = 1; - alwaysyes = alwaysno = 0; break; default: @@ -130,9 +129,10 @@ ask(int def, const char *fmt, ...) char prompt[256]; int c; + if (alwaysyes || alwaysno || rdonly) + def = (alwaysyes && !rdonly && !alwaysno); + if (preen) { - if (rdonly) - def = 0; if (def) printf("FIXED\n"); return def; @@ -141,9 +141,9 @@ ask(int def, const char *fmt, ...) va_start(ap, fmt); vsnprintf(prompt, sizeof(prompt), fmt, ap); va_end(ap); - if (alwaysyes || rdonly) { - printf("%s? %s\n", prompt, rdonly ? "no" : "yes"); - return !rdonly; + if (alwaysyes || alwaysno || rdonly) { + printf("%s? %s\n", prompt, def ? "yes" : "no"); + return def; } do { printf("%s? [yn] ", prompt);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201911300543.xAU5hPST044490>