lkml.org 
[lkml]   [2017]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.18 28/33] xfs: handle array index overrun in xfs_dir2_leaf_readbuf()
    Date
    3.18-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Eric Sandeen <sandeen@redhat.com>

    commit 023cc840b40fad95c6fe26fff1d380a8c9d45939 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: Carlos Maiolino <cmaiolino@redhat.com>
    Signed-off-by: Eric Sandeen <sandeen@redhat.com>
    Tested-by: Carlos Maiolino <cmaiolino@redhat.com>
    Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
    Reviewed-by: Bill O'Donnell <billodo@redhat.com>
    Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/xfs/xfs_dir2_readdir.c | 10 ++++++++--
    1 file changed, 8 insertions(+), 2 deletions(-)

    --- a/fs/xfs/xfs_dir2_readdir.c
    +++ b/fs/xfs/xfs_dir2_readdir.c
    @@ -422,6 +422,7 @@ xfs_dir2_leaf_readbuf(

    /*
    * Do we need more readahead?
    + * Each loop tries to process 1 full dir blk; last may be partial.
    */
    blk_start_plug(&plug);
    for (mip->ra_index = mip->ra_offset = i = 0;
    @@ -453,9 +454,14 @@ xfs_dir2_leaf_readbuf(
    }

    /*
    - * Advance offset through the mapping table.
    + * Advance offset through the mapping table, processing a full
    + * dir block even if it is fragmented into several extents.
    + * But stop if we have consumed all valid mappings, even if
    + * it's not yet a full directory block.
    */
    - for (j = 0; j < geo->fsbcount; j += length ) {
    + for (j = 0;
    + j < geo->fsbcount && mip->ra_index < mip->map_valid;
    + j += length ) {
    /*
    * The rest of this extent but not more than a dir
    * block.

    \
     
     \ /
      Last update: 2017-06-12 00:30    [W:4.250 / U:0.208 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site