libxfs fix for the dir2 rebalance bug

So this was a fun one to track down.
This bug has existed  since version 1.1 of the dir2 code.

xfs_dir2_leafn_rebalance splits a directory tree block into
2 balanced blocks and then calculates the new index in either the
old block or the new block relying on the hash value.
This doesn't work in the case of a new to be inserted elements hash value
being the same as an already existing elements hash value.

This resulted in a negative index being returned and then passed to
xfs_dir2_leafn_add, which it then used as a starting address in
the elements array.

The address (which is now pointing to somebody else's memory) was then
passed to memmove to move the tail of the array down 8 bytes.

Depending on the size of the array this would move all sorts of
possibly important info belong to somebody else 8 bytes down.

As part of the fix add a sanity check to xfs_dir2_leafn_add
to make nobody else is passing in a negative index.

diff --git a/libxfs/xfs_dir2_node.c b/libxfs/xfs_dir2_node.c
index e95bf98749833c218ef5fd754238c595c06666aa..b197c78ad54915bde3d51e95de68612b71477e42 100644 (file)
--- a/libxfs/xfs_dir2_node.c
+++ b/libxfs/xfs_dir2_node.c
@@ -177,12 +177,21 @@ xfs_dir2_leafn_add(
        mp = dp->i_mount;
        tp = args->trans;
        leaf = bp->data;
+
+       /*
+        * Quick check just to make sure we are not going to index
+        * into other peoples memory
+        */
+       if (index < 0)
+               return XFS_ERROR(EFSCORRUPTED);
+
        /*
         * If there are already the maximum number of leaf entries in
         * the block, if there are no stale entries it won't fit.
         * Caller will do a split.  If there are stale entries we'll do
         * a compact.
         */
+
        if (INT_GET(leaf->hdr.count, ARCH_CONVERT) == XFS_DIR2_MAX_LEAF_ENTS(mp)) {
                if (INT_ISZERO(leaf->hdr.stale, ARCH_CONVERT))
                        return XFS_ERROR(ENOSPC);
@@ -781,12 +790,24 @@ xfs_dir2_leafn_rebalance(
                state->inleaf = !swap;
        else
                state->inleaf =
-                       swap ^ (args->hashval < INT_GET(leaf2->ents[0].hashval, ARCH_CONVERT));
+                       swap ^ (blk1->index <= INT_GET(leaf1->hdr.count, ARCH_CONVERT));
        /*
         * Adjust the expected index for insertion.
         */
        if (!state->inleaf)
                blk2->index = blk1->index - INT_GET(leaf1->hdr.count, ARCH_CONVERT);
+       
+       /* 
+        * Finally sanity check just to make sure we are not returning a negative index 
+        */
+       if(blk2->index < 0) {
+               state->inleaf = 1;
+               blk2->index = 0;
+               cmn_err(CE_ALERT,
+                       "xfs_dir2_leafn_rebalance: picked the wrong leaf? reverting orignal leaf: "
+                       "blk1->index %d\n",
+                       blk1->index);
+       }
 }
 
 /*