From: Russell Cattelan Date: Fri, 16 Apr 2004 15:56:34 +0000 (+0000) Subject: libxfs fix for the dir2 rebalance bug X-Git-Tag: v2.7.0~111 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=516de2da7be862d33937c714a12ca36f5641a8db;p=thirdparty%2Fxfsprogs-dev.git 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 e95bf9874..b197c78ad 100644 --- 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); + } } /*