xfs
[Top] [All Lists]

REVIEW: xfs_bmap_check_leaf_extents() can reference unmapped memory

To: xfs-dev <xfs-dev@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Subject: REVIEW: xfs_bmap_check_leaf_extents() can reference unmapped memory
From: Lachlan McIlroy <lachlan@xxxxxxx>
Date: Thu, 20 Mar 2008 17:11:23 +1100
Reply-to: lachlan@xxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.12 (X11/20080213)
While investigating the extent corruption bug I ran into this bug in
debug only code.  xfs_bmap_check_leaf_extents() loops through the leaf
blocks of the extent btree checking that every extent is entirely
before the next extent.  It also compares the last extent in the previous
block to the first extent in the current block when the previous block
has been released and potentially unmapped.  So take a copy of the last
extent instead of a pointer.  Also move the last extent check out of the
loop because we only need to do it once.

Lachlan


--- fs/xfs/xfs_bmap.c_1.386 2008-03-17 13:37:32.000000000 +1100 +++ fs/xfs/xfs_bmap.c 2008-03-19 14:55:41.000000000 +1100 @@ -6194,7 +6194,7 @@ xfs_bmap_check_leaf_extents( xfs_mount_t *mp; /* file system mount structure */ __be64 *pp; /* pointer to block address */ xfs_bmbt_rec_t *ep; /* pointer to current extent */ - xfs_bmbt_rec_t *lastp; /* pointer to previous extent */ + xfs_bmbt_rec_t last; /* last extent in previous block */ xfs_bmbt_rec_t *nextp; /* pointer to next extent */ int bp_release = 0;

@@ -6264,7 +6264,6 @@ xfs_bmap_check_leaf_extents(
        /*
         * Loop over all leaf nodes checking that all extents are in the right 
order.
         */
-       lastp = NULL;
        for (;;) {
                xfs_fsblock_t   nextbno;
                xfs_extnum_t    num_recs;
@@ -6285,18 +6284,18 @@ xfs_bmap_check_leaf_extents(
                 */

                ep = XFS_BTREE_REC_ADDR(xfs_bmbt, block, 1);
+               if (i) {
+                       xfs_btree_check_rec(XFS_BTNUM_BMAP, (void *)&last,
+                               (void *)ep);
+               }
                for (j = 1; j < num_recs; j++) {
                        nextp = XFS_BTREE_REC_ADDR(xfs_bmbt, block, j + 1);
-                       if (lastp) {
-                               xfs_btree_check_rec(XFS_BTNUM_BMAP,
-                                       (void *)lastp, (void *)ep);
-                       }
                        xfs_btree_check_rec(XFS_BTNUM_BMAP, (void *)ep,
                                (void *)(nextp));
-                       lastp = ep;
                        ep = nextp;
                }

+               last = *ep;
                i += num_recs;
                if (bp_release) {
                        bp_release = 0;


<Prev in Thread] Current Thread [Next in Thread>