Author: mckusick
Date: Sat May 25 00:22:07 2019
New Revision: 348260
URL: https://svnweb.freebsd.org/changeset/base/348260

Log:
  MFC of 348074
  
  Rewrite fsck_readdir() and dircheck() for clarity and correctness.
  
  Approved by: re (gjb)

Modified:
  stable/11/sbin/fsck_ffs/dir.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sbin/fsck_ffs/dir.c
==============================================================================
--- stable/11/sbin/fsck_ffs/dir.c       Sat May 25 00:07:49 2019        
(r348259)
+++ stable/11/sbin/fsck_ffs/dir.c       Sat May 25 00:22:07 2019        
(r348260)
@@ -59,7 +59,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 *);
@@ -137,78 +137,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);
 }
 
@@ -217,65 +209,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;
+               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;
                        }
-                       if (dp->d_namlen != 0) {
-                               dp->d_namlen = 0;
-                               modified = 1;
-                       }
-                       if (dp->d_name[0] != '\0') {
-                               dp->d_name[0] = '\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
@@ -298,11 +305,9 @@ good:
                        }
                }
                
-               if (modified) {
-                       return 2;
-               }
+               if (modified)
+                       dirty(bp);
        }
-
        return (1);
 
 bad:
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to