lkml.org 
[lkml]   [2017]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 105/233] btrfs: use correct types for page indices in btrfs_page_exists_in_range
    3.16.48-rc1 review patch.  If anyone has any objections, please let me know.

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

    From: David Sterba <dsterba@suse.com>

    commit cc2b702c52094b637a351d7491ac5200331d0445 upstream.

    Variables start_idx and end_idx are supposed to hold a page index
    derived from the file offsets. The int type is not the right one though,
    offsets larger than 1 << 44 will get silently trimmed off the high bits.
    (1 << 44 is 16TiB)

    What can go wrong, if start is below the boundary and end gets trimmed:
    - if there's a page after start, we'll find it (radix_tree_gang_lookup_slot)
    - the final check "if (page->index <= end_idx)" will unexpectedly fail

    The function will return false, ie. "there's no page in the range",
    although there is at least one.

    btrfs_page_exists_in_range is used to prevent races in:

    * in hole punching, where we make sure there are not pages in the
    truncated range, otherwise we'll wait for them to finish and redo
    truncation, but we're going to replace the pages with holes anyway so
    the only problem is the intermediate state

    * lock_extent_direct: we want to make sure there are no pages before we
    lock and start DIO, to prevent stale data reads

    For practical occurence of the bug, there are several constaints. The
    file must be quite large, the affected range must cross the 16TiB
    boundary and the internal state of the file pages and pending operations
    must match. Also, we must not have started any ordered data in the
    range, otherwise we don't even reach the buggy function check.

    DIO locking tries hard in several places to avoid deadlocks with
    buffered IO and avoids waiting for ranges. The worst consequence seems
    to be stale data read.

    CC: Liu Bo <bo.li.liu@oracle.com>
    Fixes: fc4adbff823f7 ("btrfs: Drop EXTENT_UPTODATE check in hole punching and direct locking")
    Reviewed-by: Liu Bo <bo.li.liu@oracle.com>
    Signed-off-by: David Sterba <dsterba@suse.com>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    fs/btrfs/inode.c | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

    --- a/fs/btrfs/inode.c
    +++ b/fs/btrfs/inode.c
    @@ -6821,8 +6821,8 @@ bool btrfs_page_exists_in_range(struct i
    int found = false;
    void **pagep = NULL;
    struct page *page = NULL;
    - int start_idx;
    - int end_idx;
    + unsigned long start_idx;
    + unsigned long end_idx;

    start_idx = start >> PAGE_CACHE_SHIFT;

    \
     
     \ /
      Last update: 2017-09-10 01:43    [W:3.851 / U:0.384 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site