From 6069db97f679fdd1fbf875d691d8c4c0dc3d1d69 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Sun, 14 Feb 2010 12:30:30 +0000 Subject: [PATCH] Bug fixes from NetBSD - fix sign-compare issues. - ANSIfy a couple of functions. - Remove more duplicate #includes. - Memory leak found by Coverity on NetBSD. Submitted by: Pedro F. Giffuni Reviewed by: bde MFC after: 2 weeks --- sbin/fsck_msdosfs/boot.c | 34 ++++++++++++---------- sbin/fsck_msdosfs/check.c | 4 +-- sbin/fsck_msdosfs/dir.c | 50 ++++++++++++++++++++++++++------- sbin/fsck_msdosfs/dosfs.h | 12 ++++---- sbin/fsck_msdosfs/ext.h | 10 +++---- sbin/fsck_msdosfs/fat.c | 59 ++++++++++++++++++--------------------- sbin/fsck_msdosfs/main.c | 1 - 7 files changed, 100 insertions(+), 70 deletions(-) diff --git a/sbin/fsck_msdosfs/boot.c b/sbin/fsck_msdosfs/boot.c index 3a9de4c90c3f..d842943a2210 100644 --- a/sbin/fsck_msdosfs/boot.c +++ b/sbin/fsck_msdosfs/boot.c @@ -33,7 +33,6 @@ static const char rcsid[] = #include #include -#include #include #include @@ -41,16 +40,15 @@ static const char rcsid[] = #include "fsutil.h" int -readboot(dosfs, boot) - int dosfs; - struct bootblock *boot; +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 (read(dosfs, block, sizeof block) < sizeof block) { + if ((size_t)read(dosfs, block, sizeof block) != sizeof block) { perror("could not read boot block"); return FSFATAL; } @@ -154,12 +152,22 @@ readboot(dosfs, boot) } backup[65] = block[65]; /* XXX */ if (memcmp(block + 11, backup + 11, 79)) { - /* Correct? XXX */ - pfatal("backup doesn't compare to primary bootblock"); - if (alwaysno) - pfatal("\n"); - else - return FSFATAL; + /* + * 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. + */ + pfatal("backup (block %d) mismatch with primary bootblock:\n", + boot->Backup); + for (i = 11; i < 11 + 90; i++) { + if (block[i] != backup[i]) + pfatal("\ti=%d\tprimary 0x%02x\tbackup 0x%02x\n", + i, block[i], backup[i]); + } } /* Check backup FSInfo? XXX */ } @@ -223,9 +231,7 @@ readboot(dosfs, boot) } int -writefsinfo(dosfs, boot) - int dosfs; - struct bootblock *boot; +writefsinfo(int dosfs, struct bootblock *boot) { u_char fsinfo[2 * DOSBOOTBLOCKSIZE]; diff --git a/sbin/fsck_msdosfs/check.c b/sbin/fsck_msdosfs/check.c index f04f6ac9abe0..e7537ba40aa3 100644 --- a/sbin/fsck_msdosfs/check.c +++ b/sbin/fsck_msdosfs/check.c @@ -33,7 +33,6 @@ static const char rcsid[] = #include #include -#include #include #include #include @@ -47,7 +46,8 @@ checkfilesys(const char *fname) int dosfs; struct bootblock boot; struct fatEntry *fat = NULL; - int i, finish_dosdirsection=0; + int finish_dosdirsection=0; + u_int i; int mod = 0; int ret = 8; diff --git a/sbin/fsck_msdosfs/dir.c b/sbin/fsck_msdosfs/dir.c index 756d6e8a431e..515929c1e4b8 100644 --- a/sbin/fsck_msdosfs/dir.c +++ b/sbin/fsck_msdosfs/dir.c @@ -37,7 +37,6 @@ static const char rcsid[] = #include #include #include -#include #include #include @@ -223,12 +222,24 @@ resetDosDirSection(struct bootblock *boot, struct fatEntry *fat) b1 = boot->RootDirEnts * 32; b2 = boot->SecPerClust * boot->BytesPerSec; - if (!(buffer = malloc(b1 > b2 ? b1 : b2)) - || !(delbuf = malloc(b2)) - || !(rootDir = newDosDirEntry())) { - perror("No space for directory"); + if ((buffer = malloc( b1 > b2 ? b1 : b2)) == NULL) { + perror("No space for directory buffer"); return FSFATAL; } + + if ((delbuf = malloc(b2)) == NULL) { + free(buffer); + perror("No space for directory delbuf"); + return FSFATAL; + } + + if ((rootDir = newDosDirEntry()) == NULL) { + free(buffer); + free(delbuf); + perror("No space for directory entry"); + return FSFATAL; + } + memset(rootDir, 0, sizeof *rootDir); if (boot->flags & FAT32) { if (boot->RootCl < CLUST_FIRST || boot->RootCl >= boot->NumClusters) { @@ -360,7 +371,8 @@ removede(int f, struct bootblock *boot, struct fatEntry *fat, u_char *start, return FSFATAL; start = buffer; } - if (endcl == curcl) + /* startcl is < CLUST_FIRST for !fat32 root */ + if ((endcl == curcl) || (startcl < CLUST_FIRST)) for (; start < end; start += 32) *start = SLOT_DELETED; return FSDIRMOD; @@ -378,7 +390,7 @@ checksize(struct bootblock *boot, struct fatEntry *fat, u_char *p, /* * Check size on ordinary files */ - int32_t physicalSize; + u_int32_t physicalSize; if (dir->head == CLUST_FREE) physicalSize = 0; @@ -637,7 +649,8 @@ readDosDirSection(int f, struct bootblock *boot, struct fatEntry *fat, dirent.head |= (p[20] << 16) | (p[21] << 24); dirent.size = p[28] | (p[29] << 8) | (p[30] << 16) | (p[31] << 24); if (vallfn) { - strcpy(dirent.lname, longName); + strlcpy(dirent.lname, longName, + sizeof(dirent.lname)); longName[0] = '\0'; shortSum = -1; } @@ -825,6 +838,10 @@ readDosDirSection(int f, struct bootblock *boot, struct fatEntry *fat, } boot->NumFiles++; } + + if (!(boot->flags & FAT32) && !dir->parent) + break; + if (mod & THISMOD) { last *= 32; if (lseek(f, off, SEEK_SET) != off @@ -840,6 +857,19 @@ readDosDirSection(int f, struct bootblock *boot, struct fatEntry *fat, invlfn ? invlfn : vallfn, p, invlfn ? invcl : valcl, -1, 0, fullpath(dir), 1); + + /* The root directory of non fat32 filesystems is in a special + * area and may have been modified above without being written out. + */ + if ((mod & FSDIRMOD) && !(boot->flags & FAT32) && !dir->parent) { + last *= 32; + if (lseek(f, off, SEEK_SET) != off + || write(f, buffer, last) != last) { + perror("Unable to write directory"); + return FSFATAL; + } + mod &= ~THISMOD; + } return mod & ~THISMOD; } @@ -929,7 +959,7 @@ reconnect(int dosfs, struct bootblock *boot, struct fatEntry *fat, cl_t head) lfoff = lfcl * boot->ClusterSize + boot->ClusterOffset * boot->BytesPerSec; if (lseek(dosfs, lfoff, SEEK_SET) != lfoff - || read(dosfs, lfbuf, boot->ClusterSize) != boot->ClusterSize) { + || (size_t)read(dosfs, lfbuf, boot->ClusterSize) != boot->ClusterSize) { perror("could not read LOST.DIR"); return FSFATAL; } @@ -959,7 +989,7 @@ reconnect(int dosfs, struct bootblock *boot, struct fatEntry *fat, cl_t head) p[31] = (u_char)(d.size >> 24); fat[head].flags |= FAT_USED; if (lseek(dosfs, lfoff, SEEK_SET) != lfoff - || write(dosfs, lfbuf, boot->ClusterSize) != boot->ClusterSize) { + || (size_t)write(dosfs, lfbuf, boot->ClusterSize) != boot->ClusterSize) { perror("could not write LOST.DIR"); return FSFATAL; } diff --git a/sbin/fsck_msdosfs/dosfs.h b/sbin/fsck_msdosfs/dosfs.h index 4220b12b1f14..4a968d0eae4c 100644 --- a/sbin/fsck_msdosfs/dosfs.h +++ b/sbin/fsck_msdosfs/dosfs.h @@ -48,8 +48,13 @@ struct bootblock { u_int FATsmall; /* number of sectors per FAT */ u_int SecPerTrack; /* sectors per track */ u_int Heads; /* number of heads */ - u_int32_t Sectors; /* total number of sectors */ u_int32_t HiddenSecs; /* # of hidden sectors */ + u_int32_t Sectors; /* total number of sectors */ +#define FAT32 1 /* this is a FAT32 file system */ + /* + * Maybe, we should separate out + * various parts of FAT32? XXX + */ u_int32_t HugeSectors; /* # of sectors if bpbSectors == 0 */ u_int FSInfo; /* FSInfo sector */ u_int Backup; /* Backup of Bootblocks */ @@ -59,11 +64,6 @@ struct bootblock { /* and some more calculated values */ u_int flags; /* some flags: */ -#define FAT32 1 /* this is a FAT32 file system */ - /* - * Maybe, we should separate out - * various parts of FAT32? XXX - */ int ValidFat; /* valid fat if FAT32 non-mirrored */ cl_t ClustMask; /* mask for entries in FAT */ cl_t NumClusters; /* # of entries in a FAT */ diff --git a/sbin/fsck_msdosfs/ext.h b/sbin/fsck_msdosfs/ext.h index 00d028aa9718..a4fd19b8cd72 100644 --- a/sbin/fsck_msdosfs/ext.h +++ b/sbin/fsck_msdosfs/ext.h @@ -70,12 +70,12 @@ int checkfilesys(const char *); #define FSDIRMOD 2 /* Some directory was modified */ #define FSFATMOD 4 /* The FAT was modified */ #define FSERROR 8 /* Some unrecovered error remains */ -#define FSFATAL 16 /* Some unrecoverable error occured */ +#define FSFATAL 16 /* Some unrecoverable error occurred */ #define FSDIRTY 32 /* File system is dirty */ #define FSFIXFAT 64 /* Fix file system FAT */ /* - * read a boot block in a machine independend fashion and translate + * read a boot block in a machine independent fashion and translate * it into our struct bootblock. */ int readboot(int, struct bootblock *); @@ -89,13 +89,13 @@ int writefsinfo(int, struct bootblock *); * Read one of the FAT copies and return a pointer to the new * allocated array holding our description of it. */ -int readfat(int, struct bootblock *, int, struct fatEntry **); +int readfat(int, struct bootblock *, u_int, struct fatEntry **); /* * Check two FAT copies for consistency and merge changes into the - * first if neccessary. + * first if necessary. */ -int comparefat(struct bootblock *, struct fatEntry *, struct fatEntry *, int); +int comparefat(struct bootblock *, struct fatEntry *, struct fatEntry *, u_int); /* * Check a FAT diff --git a/sbin/fsck_msdosfs/fat.c b/sbin/fsck_msdosfs/fat.c index a1123f6833af..8f4bf4f40b8b 100644 --- a/sbin/fsck_msdosfs/fat.c +++ b/sbin/fsck_msdosfs/fat.c @@ -40,10 +40,10 @@ static const char rcsid[] = #include "ext.h" #include "fsutil.h" -static int checkclnum(struct bootblock *, int, cl_t, cl_t *); -static int clustdiffer(cl_t, cl_t *, cl_t *, int); +static int checkclnum(struct bootblock *, u_int, cl_t, cl_t *); +static int clustdiffer(cl_t, cl_t *, cl_t *, u_int); static int tryclear(struct bootblock *, struct fatEntry *, cl_t, cl_t *); -static int _readfat(int, struct bootblock *, int, u_char **); +static int _readfat(int, struct bootblock *, u_int, u_char **); /*- * The first 2 FAT entries contain pseudo-cluster numbers with the following @@ -128,7 +128,7 @@ checkdirty(int fs, struct bootblock *boot) * Check a cluster number for valid value */ static int -checkclnum(struct bootblock *boot, int fat, cl_t cl, cl_t *next) +checkclnum(struct bootblock *boot, u_int fat, cl_t cl, cl_t *next) { if (*next >= (CLUST_RSRVD&boot->ClustMask)) *next |= ~boot->ClustMask; @@ -159,7 +159,7 @@ checkclnum(struct bootblock *boot, int fat, cl_t cl, cl_t *next) * Read a FAT from disk. Returns 1 if successful, 0 otherwise. */ static int -_readfat(int fs, struct bootblock *boot, int no, u_char **buffer) +_readfat(int fs, struct bootblock *boot, u_int no, u_char **buffer) { off_t off; @@ -177,7 +177,7 @@ _readfat(int fs, struct bootblock *boot, int no, u_char **buffer) goto err; } - if (read(fs, *buffer, boot->FATsecs * boot->BytesPerSec) + if ((size_t)read(fs, *buffer, boot->FATsecs * boot->BytesPerSec) != boot->FATsecs * boot->BytesPerSec) { perror("Unable to read FAT"); goto err; @@ -194,24 +194,26 @@ _readfat(int fs, struct bootblock *boot, int no, u_char **buffer) * Read a FAT and decode it into internal format */ int -readfat(int fs, struct bootblock *boot, int no, struct fatEntry **fp) +readfat(int fs, struct bootblock *boot, u_int no, struct fatEntry **fp) { struct fatEntry *fat; u_char *buffer, *p; cl_t cl; int ret = FSOK; + size_t len; boot->NumFree = boot->NumBad = 0; if (!_readfat(fs, boot, no, &buffer)) return FSFATAL; - fat = calloc(boot->NumClusters, sizeof(struct fatEntry)); + fat = malloc(len = boot->NumClusters * sizeof(struct fatEntry)); if (fat == NULL) { perror("No space for FAT"); free(buffer); return FSFATAL; } + (void)memset(fat, 0, len); if (buffer[0] != boot->Media || buffer[1] != 0xff || buffer[2] != 0xff @@ -304,7 +306,11 @@ readfat(int fs, struct bootblock *boot, int no, struct fatEntry **fp) } free(buffer); - *fp = fat; + if (ret & FSFATAL) { + free(fat); + *fp = NULL; + } else + *fp = fat; return ret; } @@ -324,7 +330,7 @@ rsrvdcltype(cl_t cl) } static int -clustdiffer(cl_t cl, cl_t *cp1, cl_t *cp2, int fatnum) +clustdiffer(cl_t cl, cl_t *cp1, cl_t *cp2, u_int fatnum) { if (*cp1 == CLUST_FREE || *cp1 >= CLUST_RSRVD) { if (*cp2 == CLUST_FREE || *cp2 >= CLUST_RSRVD) { @@ -339,13 +345,13 @@ clustdiffer(cl_t cl, cl_t *cp1, cl_t *cp2, int fatnum) } return FSFATAL; } - pwarn("Cluster %u is marked %s in FAT 0, %s in FAT %d\n", + pwarn("Cluster %u is marked %s in FAT 0, %s in FAT %u\n", cl, rsrvdcltype(*cp1), rsrvdcltype(*cp2), fatnum); if (ask(0, "Use FAT 0's entry")) { *cp2 = *cp1; return FSFATMOD; } - if (ask(0, "Use FAT %d's entry", fatnum)) { + if (ask(0, "Use FAT %u's entry", fatnum)) { *cp1 = *cp2; return FSFATMOD; } @@ -353,7 +359,7 @@ clustdiffer(cl_t cl, cl_t *cp1, cl_t *cp2, int fatnum) } pwarn("Cluster %u is marked %s in FAT 0, but continues with cluster %u in FAT %d\n", cl, rsrvdcltype(*cp1), *cp2, fatnum); - if (ask(0, "Use continuation from FAT %d", fatnum)) { + if (ask(0, "Use continuation from FAT %u", fatnum)) { *cp1 = *cp2; return FSFATMOD; } @@ -364,7 +370,7 @@ clustdiffer(cl_t cl, cl_t *cp1, cl_t *cp2, int fatnum) return FSFATAL; } if (*cp2 == CLUST_FREE || *cp2 >= CLUST_RSRVD) { - pwarn("Cluster %u continues with cluster %u in FAT 0, but is marked %s in FAT %d\n", + pwarn("Cluster %u continues with cluster %u in FAT 0, but is marked %s in FAT %u\n", cl, *cp1, rsrvdcltype(*cp2), fatnum); if (ask(0, "Use continuation from FAT 0")) { *cp2 = *cp1; @@ -376,13 +382,13 @@ clustdiffer(cl_t cl, cl_t *cp1, cl_t *cp2, int fatnum) } return FSERROR; } - pwarn("Cluster %u continues with cluster %u in FAT 0, but with cluster %u in FAT %d\n", + pwarn("Cluster %u continues with cluster %u in FAT 0, but with cluster %u in FAT %u\n", cl, *cp1, *cp2, fatnum); if (ask(0, "Use continuation from FAT 0")) { *cp2 = *cp1; return FSFATMOD; } - if (ask(0, "Use continuation from FAT %d", fatnum)) { + if (ask(0, "Use continuation from FAT %u", fatnum)) { *cp1 = *cp2; return FSFATMOD; } @@ -394,8 +400,8 @@ clustdiffer(cl_t cl, cl_t *cp1, cl_t *cp2, int fatnum) * into the first one. */ int -comparefat(struct bootblock *boot, struct fatEntry *first, - struct fatEntry *second, int fatnum) +comparefat(struct bootblock *boot, struct fatEntry *first, + struct fatEntry *second, u_int fatnum) { cl_t cl; int ret = FSOK; @@ -535,8 +541,8 @@ writefat(int fs, struct bootblock *boot, struct fatEntry *fat, int correct_fat) { u_char *buffer, *p; cl_t cl; - int i; - u_int32_t fatsz; + u_int i; + size_t fatsz; off_t off; int ret = FSOK; @@ -626,7 +632,7 @@ writefat(int fs, struct bootblock *boot, struct fatEntry *fat, int correct_fat) off = boot->ResSectors + i * boot->FATsecs; off *= boot->BytesPerSec; if (lseek(fs, off, SEEK_SET) != off - || write(fs, buffer, fatsz) != fatsz) { + || (size_t)write(fs, buffer, fatsz) != fatsz) { perror("Unable to write FAT"); ret = FSFATAL; /* Return immediately? XXX */ } @@ -676,17 +682,6 @@ checklost(int dosfs, struct bootblock *boot, struct fatEntry *fat) ret = 1; } } - if (boot->NumFree && fat[boot->FSNext].next != CLUST_FREE) { - pwarn("Next free cluster in FSInfo block (%u) not free\n", - boot->FSNext); - 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); } diff --git a/sbin/fsck_msdosfs/main.c b/sbin/fsck_msdosfs/main.c index e23d031e8449..9ec15ca2b500 100644 --- a/sbin/fsck_msdosfs/main.c +++ b/sbin/fsck_msdosfs/main.c @@ -33,7 +33,6 @@ static const char rcsid[] = #include #include -#include #include #include #include