[src/trunk]: src/sys/ufs/ext2fs Fix ext2fs's scary cross-block directory mess...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/fb1d4663e516
branches:  trunk
changeset: 779591:fb1d4663e516
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Mon Jun 04 19:45:59 2012 +0000

description:
Fix ext2fs's scary cross-block directory message too.

(See rev. 1.3 of sys/ufs/ufs/ufs_rename.c for the analysis.)

diffstat:

 sys/ufs/ext2fs/ext2fs_rename.c |  87 +++++++++++++++++++----------------------
 1 files changed, 41 insertions(+), 46 deletions(-)

diffs (166 lines):

diff -r ff878c29e124 -r fb1d4663e516 sys/ufs/ext2fs/ext2fs_rename.c
--- a/sys/ufs/ext2fs/ext2fs_rename.c    Mon Jun 04 19:45:50 2012 +0000
+++ b/sys/ufs/ext2fs/ext2fs_rename.c    Mon Jun 04 19:45:59 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ext2fs_rename.c,v 1.2 2012/05/10 19:08:34 riastradh Exp $      */
+/*     $NetBSD: ext2fs_rename.c,v 1.3 2012/06/04 19:45:59 riastradh Exp $      */
 
 /*-
  * Copyright (c) 2012 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ext2fs_rename.c,v 1.2 2012/05/10 19:08:34 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ext2fs_rename.c,v 1.3 2012/06/04 19:45:59 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/buf.h>
@@ -487,10 +487,9 @@
         * inserting a new entry.  That may invalidate fulr, which we
         * need in order to remove the old entry.  In that case, we
         * need to recalculate what fulr should be.
-        *
-        * XXX I believe this is necessary only if tvp == NULL as well.
         */
-       if (!reparent_p && ext2fs_rename_ulr_overlap_p(fulr, tulr)) {
+       if (!reparent_p && (tvp == NULL) &&
+           ext2fs_rename_ulr_overlap_p(fulr, tulr)) {
                error = ext2fs_rename_recalculate_fulr(fdvp, fulr, tulr, fcnp);
 #if 0                          /* XXX */
                if (error)      /* XXX Try to back out changes?  */
@@ -571,13 +570,11 @@
        struct mount *mp;
        struct ufsmount *ump;
        /* XXX int is a silly type for this; blame ufsmount::um_dirblksiz.  */
-       int directory_block_mask;
-       unsigned long io_block_mask;
+       int dirblksiz;
+       doff_t search_start, search_end;
        doff_t offset;          /* Offset of entry we're examining.  */
-       doff_t search_end;      /* Limit to our search.  */
        struct buf *bp;         /* I/O block we're examining.  */
-       char *dirbuf;           /* Pointer into bp's data.  */
-       doff_t dirbuf_offset;   /* Offset of dirbuf from directory start.  */
+       char *dirbuf;           /* Pointer into directory at search_start.  */
        struct ext2fs_direct *ep; /* Pointer to the entry we're examining.  */
        /* XXX direct::d_reclen is 16-bit;
         * ufs_lookup_results::ulr_reclen is 32-bit.  Blah.  */
@@ -598,66 +595,55 @@
        KASSERT(ump != NULL);
        KASSERT(ump == VTOI(dvp)->i_ump);
 
-       KASSERT(0 < ump->um_dirblksiz);
-       KASSERT((ump->um_dirblksiz & (ump->um_dirblksiz - 1)) == 0);
-       directory_block_mask = (ump->um_dirblksiz - 1);
+       dirblksiz = ump->um_dirblksiz;
+       KASSERT(0 < dirblksiz);
+       KASSERT((dirblksiz & (dirblksiz - 1)) == 0);
 
-       KASSERT(0 < mp->mnt_stat.f_iosize);
-       KASSERT((mp->mnt_stat.f_iosize & (mp->mnt_stat.f_iosize - 1)) == 0);
-       io_block_mask = (mp->mnt_stat.f_iosize - 1);
+       /* A directory block may not span across multiple I/O blocks.  */
+       KASSERT(dirblksiz <= mp->mnt_stat.f_iosize);
 
-       offset = tulr->ulr_offset;
+       /* Find the bounds of the search.  */
+       search_start = tulr->ulr_offset;
        KASSERT(fulr->ulr_reclen < (EXT2FS_MAXDIRSIZE - fulr->ulr_offset));
        search_end = (fulr->ulr_offset + fulr->ulr_reclen);
 
+       /* Compaction must happen only within a directory block. (*)  */
+       KASSERT(search_start <= search_end);
+       KASSERT((search_end - (search_start &~ (dirblksiz - 1))) <= dirblksiz);
+
        dirbuf = NULL;
        bp = NULL;
-       dirbuf_offset = offset;
-       error = ext2fs_blkatoff(dvp, (off_t)dirbuf_offset, &dirbuf, &bp);
+       error = ext2fs_blkatoff(dvp, (off_t)search_start, &dirbuf, &bp);
        if (error)
                return error;
        KASSERT(dirbuf != NULL);
        KASSERT(bp != NULL);
 
+       /*
+        * Guarantee we sha'n't go past the end of the buffer we got.
+        * dirbuf is bp->b_data + (search_start & (iosize - 1)), and
+        * the valid range is [bp->b_data, bp->b_data + bp->b_bcount).
+        */
+       KASSERT((search_end - search_start) <=
+           (bp->b_bcount - (search_start & (mp->mnt_stat.f_iosize - 1))));
+
        prev_reclen = fulr->ulr_count;
+       offset = search_start;
 
        /*
-        * Search from offset to search_end for the entry matching
+        * Search from search_start to search_end for the entry matching
         * fcnp, which must be there because we found it before and it
         * should only at most have moved earlier.
         */
        for (;;) {
+               KASSERT(search_start <= offset);
                KASSERT(offset < search_end);
 
                /*
-                * If we are at an I/O block boundary, fetch the next block.
-                */
-               if ((offset & io_block_mask) == 0) {
-#ifdef DIAGNOSTIC              /* XXX */
-                       printf("%s: directory block of inode 0x%llx"
-                           " extends across I/O block boundary,"
-                           " which shouldn't happen!\n",
-                           mp->mnt_stat.f_mntonname,
-                           (unsigned long long)VTOI(dvp)->i_number);
-#endif
-                       brelse(bp, 0);
-                       dirbuf = NULL;
-                       bp = NULL;
-                       dirbuf_offset = offset;
-                       error = ext2fs_blkatoff(dvp, (off_t)dirbuf_offset,
-                           &dirbuf, &bp);
-                       if (error)
-                               return error;
-                       KASSERT(dirbuf != NULL);
-                       KASSERT(bp != NULL);
-               }
-
-               /*
                 * Examine the directory entry at offset.
                 */
-               KASSERT(dirbuf_offset <= offset);
                ep = (struct ext2fs_direct *)
-                   (dirbuf + (offset - dirbuf_offset));
+                   (dirbuf + (offset - search_start));
                reclen = fs2h16(ep->e2d_reclen);
 
                if (ep->e2d_ino == 0)
@@ -682,8 +668,17 @@
                        return EIO;     /* XXX Panic?  What?  */
                }
 
+               /* We may not move past the search end.  */
                KASSERT(reclen < search_end);
                KASSERT(offset < (search_end - reclen));
+
+               /*
+                * We may not move across a directory block boundary;
+                * see (*) above.
+                */
+               KASSERT((offset &~ (dirblksiz - 1)) ==
+                   ((offset + reclen) &~ (dirblksiz - 1)));
+
                prev_reclen = reclen;
                offset += reclen;
        }
@@ -698,7 +693,7 @@
         * Record the preceding record length, but not if we're at the
         * start of a directory block.
         */
-       fulr->ulr_count = ((offset & directory_block_mask)? prev_reclen : 0);
+       fulr->ulr_count = ((offset & (dirblksiz - 1))? prev_reclen : 0);
 
        brelse(bp, 0);
        return 0;