• Eric Sandeen's avatar
    xfs: handle array index overrun in xfs_dir2_leaf_readbuf() · c4d3116e
    Eric Sandeen authored
    commit 023cc840 upstream.
    
    Carlos had a case where "find" seemed to start spinning
    forever and never return.
    
    This was on a filesystem with non-default multi-fsb (8k)
    directory blocks, and a fragmented directory with extents
    like this:
    
    0:[0,133646,2,0]
    1:[2,195888,1,0]
    2:[3,195890,1,0]
    3:[4,195892,1,0]
    4:[5,195894,1,0]
    5:[6,195896,1,0]
    6:[7,195898,1,0]
    7:[8,195900,1,0]
    8:[9,195902,1,0]
    9:[10,195908,1,0]
    10:[11,195910,1,0]
    11:[12,195912,1,0]
    12:[13,195914,1,0]
    ...
    
    i.e. the first extent is a contiguous 2-fsb dir block, but
    after that it is fragmented into 1 block extents.
    
    At the top of the readdir path, we allocate a mapping array
    which (for this filesystem geometry) can hold 10 extents; see
    the assignment to map_info->map_size.  During readdir, we are
    therefore able to map extents 0 through 9 above into the array
    for readahead purposes.  If we count by 2, we see that the last
    mapped index (9) is the first block of a 2-fsb directory block.
    
    At the end of xfs_dir2_leaf_readbuf() we have 2 loops to fill
    more readahead; the outer loop assumes one full dir block is
    processed each loop iteration, and an inner loop that ensures
    that this is so by advancing to the next extent until a full
    directory block is mapped.
    
    The problem is that this inner loop may step past the last
    extent in the mapping array as it tries to reach the end of
    the directory block.  This will read garbage for the extent
    length, and as a result the loop control variable 'j' may
    become corrupted and never fail the loop conditional.
    
    The number of valid mappings we have in our array is stored
    in map->map_valid, so stop this inner loop based on that limit.
    
    There is an ASSERT at the top of the outer loop for this
    same condition, but we never made it out of the inner loop,
    so the ASSERT never fired.
    
    Huge appreciation for Carlos for debugging and isolating
    the problem.
    Debugged-and-analyzed-by: default avatarCarlos Maiolino <cmaiolino@redhat.com>
    Signed-off-by: default avatarEric Sandeen <sandeen@redhat.com>
    Tested-by: default avatarCarlos Maiolino <cmaiolino@redhat.com>
    Reviewed-by: default avatarCarlos Maiolino <cmaiolino@redhat.com>
    Reviewed-by: default avatarBill O'Donnell <billodo@redhat.com>
    Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    c4d3116e
xfs_dir2_readdir.c 18.5 KB