• Darrick J. Wong's avatar
    xfs: _{attr,data}_map_shared should take ILOCK_EXCL until iread_extents is completely done · c95356ca
    Darrick J. Wong authored
    While fuzzing the data fork extent count on a btree-format directory
    with xfs/375, I observed the following (excerpted) splat:
    
    XFS: Assertion failed: xfs_isilocked(ip, XFS_ILOCK_EXCL), file: fs/xfs/libxfs/xfs_bmap.c, line: 1208
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 43192 at fs/xfs/xfs_message.c:104 assfail+0x46/0x4a [xfs]
    Call Trace:
     <TASK>
     xfs_iread_extents+0x1af/0x210 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     xchk_dir_walk+0xb8/0x190 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     xchk_parent_count_parent_dentries+0x41/0x80 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     xchk_parent_validate+0x199/0x2e0 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     xchk_parent+0xdf/0x130 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     xfs_scrub_metadata+0x2b8/0x730 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     xfs_scrubv_metadata+0x38b/0x4d0 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     xfs_ioc_scrubv_metadata+0x111/0x160 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     xfs_file_ioctl+0x367/0xf50 [xfs 09f66509ece4938760fac7de64732a0cbd3e39cd]
     __x64_sys_ioctl+0x82/0xa0
     do_syscall_64+0x2b/0x80
     entry_SYSCALL_64_after_hwframe+0x46/0xb0
    
    The cause of this is a race condition in xfs_ilock_data_map_shared,
    which performs an unlocked access to the data fork to guess which lock
    mode it needs:
    
    Thread 0                          Thread 1
    
    xfs_need_iread_extents
    <observe no iext tree>
    xfs_ilock(..., ILOCK_EXCL)
    xfs_iread_extents
    <observe no iext tree>
    <check ILOCK_EXCL>
    <load bmbt extents into iext>
    <notice iext size doesn't
     match nextents>
                                      xfs_need_iread_extents
                                      <observe iext tree>
                                      xfs_ilock(..., ILOCK_SHARED)
    <tear down iext tree>
    xfs_iunlock(..., ILOCK_EXCL)
                                      xfs_iread_extents
                                      <observe no iext tree>
                                      <check ILOCK_EXCL>
                                      *BOOM*
    
    Fix this race by adding a flag to the xfs_ifork structure to indicate
    that we have not yet read in the extent records and changing the
    predicate to look at the flag state, not if_height.  The memory barrier
    ensures that the flag will not be set until the very end of the
    function.
    Signed-off-by: default avatarDarrick J. Wong <djwong@kernel.org>
    Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
    Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
    c95356ca
xfs_inode_fork.h 7.87 KB