aboutsummaryrefslogtreecommitdiffstats
path: root/sbin/fsck_ffs
diff options
context:
space:
mode:
authorKirk McKusick <mckusick@FreeBSD.org>2019-05-21 22:24:38 +0000
committerKirk McKusick <mckusick@FreeBSD.org>2019-05-21 22:24:38 +0000
commitbfc5d3f9c267193574a269d9d265021980292072 (patch)
tree7c0c52f2b8484bb94c2f3bde52a588f5159039b4 /sbin/fsck_ffs
parent34841dd627d60ee7a3986d3d56b7cb98231f5dd7 (diff)
downloadsrc-bfc5d3f9c267193574a269d9d265021980292072.tar.gz
src-bfc5d3f9c267193574a269d9d265021980292072.zip
This revision began as a simple change to eliminate an uninitialized warning
found by Coverity. However, upon closer inspection the implementation of fsck_ffs's fsck_readdir() and dircheck() functions is both nearly impossible to follow and fails to check / fix directories in several cases. So, this revision is an entire rewrite of these two functions to clarify what they are doing and also to get something that works properly. Referred by: cem Reviewed by: kib, David G Lawrence MFC after: 3 days CID 1401317: namlen may be used uninitialized
Notes
Notes: svn path=/head/; revision=348074
Diffstat (limited to 'sbin/fsck_ffs')
-rw-r--r--sbin/fsck_ffs/dir.c205
1 files changed, 105 insertions, 100 deletions
diff --git a/sbin/fsck_ffs/dir.c b/sbin/fsck_ffs/dir.c
index b39df6d61a13..831cf448527b 100644
--- a/sbin/fsck_ffs/dir.c
+++ b/sbin/fsck_ffs/dir.c
@@ -61,7 +61,7 @@ static struct dirtemplate dirhead = {
};
static int chgino(struct inodesc *);
-static int dircheck(struct inodesc *, struct direct *);
+static int dircheck(struct inodesc *, struct bufarea *, struct direct *);
static int expanddir(union dinode *dp, char *name);
static void freedir(ino_t ino, ino_t parent);
static struct direct *fsck_readdir(struct inodesc *);
@@ -139,78 +139,70 @@ dirscan(struct inodesc *idesc)
}
/*
- * get next entry in a directory.
+ * Get and verify the next entry in a directory.
+ * We also verify that if there is another entry in the block that it is
+ * valid, so if it is not valid it can be subsumed into the current entry.
*/
static struct direct *
fsck_readdir(struct inodesc *idesc)
{
struct direct *dp, *ndp;
struct bufarea *bp;
- long size, blksiz, fix, dploc;
- int dc;
+ long size, blksiz, subsume_ndp;
+ subsume_ndp = 0;
blksiz = idesc->id_numfrags * sblock.fs_fsize;
+ if (idesc->id_filesize <= 0 || idesc->id_loc >= blksiz)
+ return (NULL);
bp = getdirblk(idesc->id_blkno, blksiz);
- if (idesc->id_loc % DIRBLKSIZ == 0 && idesc->id_filesize > 0 &&
- idesc->id_loc < blksiz) {
- dp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc);
- if ((dc = dircheck(idesc, dp)) > 0) {
- if (dc == 2) {
- /*
- * dircheck() cleared unused directory space.
- * Mark the buffer as dirty to write it out.
- */
- dirty(bp);
- }
- goto dpok;
- }
- if (idesc->id_fix == IGNORE)
- return (0);
- fix = dofix(idesc, "DIRECTORY CORRUPTED");
- bp = getdirblk(idesc->id_blkno, blksiz);
- dp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc);
- dp->d_reclen = DIRBLKSIZ;
- dp->d_ino = 0;
- dp->d_type = 0;
- dp->d_namlen = 0;
- dp->d_name[0] = '\0';
- if (fix)
- dirty(bp);
- idesc->id_loc += DIRBLKSIZ;
- idesc->id_filesize -= DIRBLKSIZ;
- return (dp);
+ dp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc);
+ /*
+ * Only need to check current entry if it is the first in the
+ * the block, as later entries will have been checked in the
+ * previous call to this function.
+ */
+ if (idesc->id_loc % DIRBLKSIZ != 0 || dircheck(idesc, bp, dp) != 0) {
+ /*
+ * Current entry is good, update to point at next.
+ */
+ idesc->id_loc += dp->d_reclen;
+ idesc->id_filesize -= dp->d_reclen;
+ /*
+ * If at end of directory block, just return this entry.
+ */
+ if (idesc->id_filesize <= 0 || idesc->id_loc >= blksiz ||
+ idesc->id_loc % DIRBLKSIZ == 0)
+ return (dp);
+ /*
+ * If the next entry good, return this entry.
+ */
+ ndp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc);
+ if (dircheck(idesc, bp, ndp) != 0)
+ return (dp);
+ /*
+ * The next entry is bad, so subsume it and the remainder
+ * of this directory block into this entry.
+ */
+ subsume_ndp = 1;
}
-dpok:
- if (idesc->id_filesize <= 0 || idesc->id_loc >= blksiz)
- return NULL;
- dploc = idesc->id_loc;
- dp = (struct direct *)(bp->b_un.b_buf + dploc);
- idesc->id_loc += dp->d_reclen;
- idesc->id_filesize -= dp->d_reclen;
- if ((idesc->id_loc % DIRBLKSIZ) == 0)
- return (dp);
- ndp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc);
- if (idesc->id_loc < blksiz && idesc->id_filesize > 0) {
- if ((dc = dircheck(idesc, ndp)) == 0) {
- size = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ);
- idesc->id_loc += size;
- idesc->id_filesize -= size;
- if (idesc->id_fix == IGNORE)
- return (0);
- fix = dofix(idesc, "DIRECTORY CORRUPTED");
- bp = getdirblk(idesc->id_blkno, blksiz);
- dp = (struct direct *)(bp->b_un.b_buf + dploc);
- dp->d_reclen += size;
- if (fix)
- dirty(bp);
- } else if (dc == 2) {
- /*
- * dircheck() cleared unused directory space.
- * Mark the buffer as dirty to write it out.
- */
- dirty(bp);
- }
+ /*
+ * Current or next entry is bad. Zap current entry or
+ * subsume next entry into current entry as appropriate.
+ */
+ size = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ);
+ idesc->id_loc += size;
+ idesc->id_filesize -= size;
+ if (idesc->id_fix == IGNORE)
+ return (NULL);
+ if (subsume_ndp) {
+ memset(ndp, 0, size);
+ dp->d_reclen += size;
+ } else {
+ memset(dp, 0, size);
+ dp->d_reclen = size;
}
+ if (dofix(idesc, "DIRECTORY CORRUPTED"))
+ dirty(bp);
return (dp);
}
@@ -219,65 +211,80 @@ dpok:
* This is a superset of the checks made in the kernel.
* Also optionally clears padding and unused directory space.
*
- * Returns 0 if the entry is bad, 1 if the entry is good and no changes
- * were made, and 2 if the entry is good but modified to clear out padding
- * and unused space and needs to be written back to disk.
+ * Returns 0 if the entry is bad, 1 if the entry is good.
*/
static int
-dircheck(struct inodesc *idesc, struct direct *dp)
+dircheck(struct inodesc *idesc, struct bufarea *bp, struct direct *dp)
{
size_t size;
char *cp;
- u_char type;
u_int8_t namlen;
int spaceleft, modified, unused;
- modified = 0;
spaceleft = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ);
+ size = DIRSIZ(0, dp);
if (dp->d_reclen == 0 ||
dp->d_reclen > spaceleft ||
+ dp->d_reclen < size ||
+ idesc->id_filesize < size ||
(dp->d_reclen & (DIR_ROUNDUP - 1)) != 0)
goto bad;
+ modified = 0;
if (dp->d_ino == 0) {
+ if (!zflag || fswritefd < 0)
+ return (1);
/*
- * Special case of an unused directory entry. Normally
- * the kernel would coalesce unused space with the previous
- * entry by extending its d_reclen, but there are situations
- * (e.g. fsck) where that doesn't occur.
- * If we're clearing out directory cruft (-z flag), then make
- * sure this entry gets fully cleared as well.
+ * Special case of an unused directory entry. Normally only
+ * occurs at the beginning of a directory block when the block
+ * contains no entries. Other than the first entry in a
+ * directory block, the kernel coalesces unused space with
+ * the previous entry by extending its d_reclen. However,
+ * when cleaning up a directory, fsck may set d_ino to zero
+ * in the middle of a directory block. If we're clearing out
+ * directory cruft (-z flag), then make sure that all directory
+ * space in entries with d_ino == 0 gets fully cleared.
*/
- if (zflag && fswritefd >= 0) {
- if (dp->d_type != 0) {
- dp->d_type = 0;
- modified = 1;
- }
- if (dp->d_namlen != 0) {
- dp->d_namlen = 0;
- modified = 1;
- }
- if (dp->d_name[0] != '\0') {
- dp->d_name[0] = '\0';
+ if (dp->d_type != 0) {
+ dp->d_type = 0;
+ modified = 1;
+ }
+ if (dp->d_namlen != 0) {
+ dp->d_namlen = 0;
+ modified = 1;
+ }
+ unused = dp->d_reclen - __offsetof(struct direct, d_name);
+ for (cp = dp->d_name; unused > 0; unused--, cp++) {
+ if (*cp != '\0') {
+ *cp = '\0';
modified = 1;
}
}
- goto good;
+ if (modified)
+ dirty(bp);
+ return (1);
}
- size = DIRSIZ(0, dp);
+ /*
+ * The d_type field should not be tested here. A bad type is an error
+ * in the entry itself but is not a corruption of the directory
+ * structure itself. So blowing away all the remaining entries in the
+ * directory block is inappropriate. Rather the type error should be
+ * checked in pass1 and fixed there.
+ *
+ * The name validation should also be done in pass1 although the
+ * check to see if the name is longer than fits in the space
+ * allocated for it (i.e., the *cp != '\0' fails after exiting the
+ * loop below) then it really is a structural error that requires
+ * the stronger action taken here.
+ */
namlen = dp->d_namlen;
- type = dp->d_type;
- if (dp->d_reclen < size ||
- idesc->id_filesize < size ||
- namlen == 0 ||
- type > 15)
+ if (namlen == 0 || dp->d_type > 15)
goto bad;
- for (cp = dp->d_name, size = 0; size < namlen; size++)
- if (*cp == '\0' || (*cp++ == '/'))
+ for (cp = dp->d_name, size = 0; size < namlen; size++) {
+ if (*cp == '\0' || *cp++ == '/')
goto bad;
+ }
if (*cp != '\0')
goto bad;
-
-good:
if (zflag && fswritefd >= 0) {
/*
* Clear unused directory entry space, including the d_name
@@ -300,11 +307,9 @@ good:
}
}
- if (modified) {
- return 2;
- }
+ if (modified)
+ dirty(bp);
}
-
return (1);
bad: