afs: Fix directory entry size calculation

The number of dirent records used by an AFS directory entry should be
calculated using the assumption that there is a 16-byte name field in the
first block, rather than a 20-byte name field (which is actually the case).
This miscalculation is historic and effectively standard, so we have to use
it.

The calculation we need to use is:

	1 + (((strlen(name) + 1) + 15) >> 5)

where we are adding one to the strlen() result to account for the NUL
termination.

Fix this by the following means:

 (1) Create an inline function to do the calculation for a given name
     length.

 (2) Use the function to calculate the number of records used for a dirent
     in afs_dir_iterate_block().

     Use this to move the over-end check out of the loop since it only
     needs to be done once.

     Further use this to only go through the loop for the 2nd+ records
     composing an entry.  The only test there now is for if the record is
     allocated - and we already checked the first block at the top of the
     outer loop.

 (3) Add a max name length check in afs_dir_iterate_block().

 (4) Make afs_edit_dir_add() and afs_edit_dir_remove() use the function
     from (1) to calculate the number of blocks rather than doing it
     incorrectly themselves.

Fixes: 63a4681ff39c ("afs: Locally edit directory data for mkdir/create/unlink/...")
Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Marc Dionne <marc.dionne@auristor.com>

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 9068d5578a26f9a6c1fe51a37599af0ff9c08e66..7bd659ad959ec0eb61046e4eaea83b220ca48aaf 100644 (file)
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -350,7 +350,7 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
                                 unsigned blkoff)
 {
        union afs_xdr_dirent *dire;
-       unsigned offset, next, curr;
+       unsigned offset, next, curr, nr_slots;
        size_t nlen;
        int tmp;
 
@@ -363,13 +363,12 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
             offset < AFS_DIR_SLOTS_PER_BLOCK;
             offset = next
             ) {
-               next = offset + 1;
-
                /* skip entries marked unused in the bitmap */
                if (!(block->hdr.bitmap[offset / 8] &
                      (1 << (offset % 8)))) {
                        _debug("ENT[%zu.%u]: unused",
                               blkoff / sizeof(union afs_xdr_dir_block), offset);
+                       next = offset + 1;
                        if (offset >= curr)
                                ctx->pos = blkoff +
                                        next * sizeof(union afs_xdr_dirent);
@@ -381,35 +380,39 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
                nlen = strnlen(dire->u.name,
                               sizeof(*block) -
                               offset * sizeof(union afs_xdr_dirent));
+               if (nlen > AFSNAMEMAX - 1) {
+                       _debug("ENT[%zu]: name too long (len %u/%zu)",
+                              blkoff / sizeof(union afs_xdr_dir_block),
+                              offset, nlen);
+                       return afs_bad(dvnode, afs_file_error_dir_name_too_long);
+               }
 
                _debug("ENT[%zu.%u]: %s %zu \"%s\"",
                       blkoff / sizeof(union afs_xdr_dir_block), offset,
                       (offset < curr ? "skip" : "fill"),
                       nlen, dire->u.name);
 
-               /* work out where the next possible entry is */
-               for (tmp = nlen; tmp > 15; tmp -= sizeof(union afs_xdr_dirent)) {
-                       if (next >= AFS_DIR_SLOTS_PER_BLOCK) {
-                               _debug("ENT[%zu.%u]:"
-                                      " %u travelled beyond end dir block"
-                                      " (len %u/%zu)",
-                                      blkoff / sizeof(union afs_xdr_dir_block),
-                                      offset, next, tmp, nlen);
-                               return afs_bad(dvnode, afs_file_error_dir_over_end);
-                       }
-                       if (!(block->hdr.bitmap[next / 8] &
-                             (1 << (next % 8)))) {
-                               _debug("ENT[%zu.%u]:"
-                                      " %u unmarked extension (len %u/%zu)",
+               nr_slots = afs_dir_calc_slots(nlen);
+               next = offset + nr_slots;
+               if (next > AFS_DIR_SLOTS_PER_BLOCK) {
+                       _debug("ENT[%zu.%u]:"
+                              " %u extends beyond end dir block"
+                              " (len %zu)",
+                              blkoff / sizeof(union afs_xdr_dir_block),
+                              offset, next, nlen);
+                       return afs_bad(dvnode, afs_file_error_dir_over_end);
+               }
+
+               /* Check that the name-extension dirents are all allocated */
+               for (tmp = 1; tmp < nr_slots; tmp++) {
+                       unsigned int ix = offset + tmp;
+                       if (!(block->hdr.bitmap[ix / 8] & (1 << (ix % 8)))) {
+                               _debug("ENT[%zu.u]:"
+                                      " %u unmarked extension (%u/%u)",
                                       blkoff / sizeof(union afs_xdr_dir_block),
-                                      offset, next, tmp, nlen);
+                                      offset, tmp, nr_slots);
                                return afs_bad(dvnode, afs_file_error_dir_unmarked_ext);
                        }
-
-                       _debug("ENT[%zu.%u]: ext %u/%zu",
-                              blkoff / sizeof(union afs_xdr_dir_block),
-                              next, tmp, nlen);
-                       next++;
                }
 
                /* skip if starts before the current position */

diff --git a/fs/afs/dir_edit.c b/fs/afs/dir_edit.c
index 2ffe09abae7fcca86bf66471eb80a91e580bcb2e..f4600c1353adf79fa002c8864ae574bee9c694ec 100644 (file)
--- a/fs/afs/dir_edit.c
+++ b/fs/afs/dir_edit.c
@@ -215,8 +215,7 @@ void afs_edit_dir_add(struct afs_vnode *vnode,
        }
 
        /* Work out how many slots we're going to need. */
-       need_slots = round_up(12 + name->len + 1 + 4, AFS_DIR_DIRENT_SIZE);
-       need_slots /= AFS_DIR_DIRENT_SIZE;
+       need_slots = afs_dir_calc_slots(name->len);
 
        meta_page = kmap(page0);
        meta = &meta_page->blocks[0];
@@ -393,8 +392,7 @@ void afs_edit_dir_remove(struct afs_vnode *vnode,
        }
 
        /* Work out how many slots we're going to discard. */
-       need_slots = round_up(12 + name->len + 1 + 4, AFS_DIR_DIRENT_SIZE);
-       need_slots /= AFS_DIR_DIRENT_SIZE;
+       need_slots = afs_dir_calc_slots(name->len);
 
        meta_page = kmap(page0);
        meta = &meta_page->blocks[0];

diff --git a/fs/afs/xdr_fs.h b/fs/afs/xdr_fs.h
index c926430fd08ad441994f885339a0da10093f98a1..8ca8681645077d0bd86ff748462328a8718c1735 100644 (file)
--- a/fs/afs/xdr_fs.h
+++ b/fs/afs/xdr_fs.h
@@ -58,7 +58,8 @@ union afs_xdr_dirent {
                /* When determining the number of dirent slots needed to
                 * represent a directory entry, name should be assumed to be 16
                 * bytes, due to a now-standardised (mis)calculation, but it is
-                * in fact 20 bytes in size.
+                * in fact 20 bytes in size.  afs_dir_calc_slots() should be
+                * used for this.
                 *
                 * For names longer than (16 or) 20 bytes, extra slots should
                 * be annexed to this one using the extended_name format.
@@ -101,4 +102,15 @@ struct afs_xdr_dir_page {
        union afs_xdr_dir_block blocks[AFS_DIR_BLOCKS_PER_PAGE];
 };
 
+/*
+ * Calculate the number of dirent slots required for any given name length.
+ * The calculation is made assuming the part of the name in the first slot is
+ * 16 bytes, rather than 20, but this miscalculation is now standardised.
+ */
+static inline unsigned int afs_dir_calc_slots(size_t name_len)
+{
+       name_len++; /* NUL-terminated */
+       return 1 + ((name_len + 15) / AFS_DIR_DIRENT_SIZE);
+}
+
 #endif /* XDR_FS_H */

diff --git a/include/trace/events/afs.h b/include/trace/events/afs.h
index 4eef374d4413e2ce79254846fafda749be3a05ce..4a5cc8c64be3470b3ecf18b6d7aec66233089f07 100644 (file)
--- a/include/trace/events/afs.h
+++ b/include/trace/events/afs.h
@@ -231,6 +231,7 @@ enum afs_file_error {
        afs_file_error_dir_bad_magic,
        afs_file_error_dir_big,
        afs_file_error_dir_missing_page,
+       afs_file_error_dir_name_too_long,
        afs_file_error_dir_over_end,
        afs_file_error_dir_small,
        afs_file_error_dir_unmarked_ext,
@@ -488,6 +489,7 @@ enum afs_cb_break_reason {
        EM(afs_file_error_dir_bad_magic,        "DIR_BAD_MAGIC")        \
        EM(afs_file_error_dir_big,              "DIR_BIG")              \
        EM(afs_file_error_dir_missing_page,     "DIR_MISSING_PAGE")     \
+       EM(afs_file_error_dir_name_too_long,    "DIR_NAME_TOO_LONG")    \
        EM(afs_file_error_dir_over_end,         "DIR_ENT_OVER_END")     \
        EM(afs_file_error_dir_small,            "DIR_SMALL")            \
        EM(afs_file_error_dir_unmarked_ext,     "DIR_UNMARKED_EXT")     \