Re: [PATCH V3] xfs_repair: test for bad level in dir2 node

Mark Tinguely
2013-09-18T18:48:44Z

On 09/12/13 15:56, Eric Sandeen wrote:
> In traverse_int_dir2block(), the variable 'i' is the level in
> the tree, with 0 being a leaf node. In the "do" loop we
> start at the root, and work our way down to a leaf.
>
> If the first node we read is an interior node with NODE_MAGIC,
> but it tells us that its level is 0 (a leaf), this is clearly
> an inconsistency.
>
> Worse, we'd return with success, bno set, and only level[0]
> in the cursor initialized. Then down this path we'll
> segfault when accessing an uninitialized (and zeroed) member
> of the cursor's level array:
>
> process_node_dir2
> traverse_int_dir2block // returns 0 w/ bno set, only level[0] init'd
> process_leaf_level_dir2
> verify_dir2_path(mp, da_cursor, 0) // p_level == 0
> this_level = p_level + 1;
> node = cursor->level[this_level].bp->b_addr; // level[1] uninit& 0'd
>
> Fix this by recognizing that an interior node w/ level 0 is invalid, and
> error out as for other inconsistencies.
>
> By the time the level 0 test is done, we have already ensured that
> this block has XFS_DA[3]_NODE_MAGIC.
>
> Reported-by: Jan Yves Brueckner<jyb@gmx.com>
> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
> ---
>
> V3: Simplify the test.
>
> Mark, Dave, I know you had some concerns about other conditions being
> tested, but I think those are separate from this fix, which simply ensures
> that the level we find for this _NODE block is within the valid range
> for a node. (It also matches the test currently present in xfs_check).
>
> If we've got other missing conditions, those can be other patches,
> I think.
>
> V2: Drop re-test of hdr magic which is guaranteed to be NODE at this point.
> fix "interior inode" - s/b "interior node"
>
> My only testcase for this is Jan Yves Brueckner's badly corrupted
> filesystem image. With this change, we get i.e. :
>
> bad level in interior inode for directory inode 39869938
> corrupt block 6 in directory inode 39869957
> will junk block

I okay with this to fix the bug. I will make a note to think more on the
level == 1 case, but that is not related to the bug.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>