1. 13 7月, 2022 2 次提交
  2. 10 7月, 2022 3 次提交
    • D
      xfs: use XFS_IFORK_Q to determine the presence of an xattr fork · e45d7cb2
      Darrick J. Wong 提交于
      Modify xfs_ifork_ptr to return a NULL pointer if the caller asks for the
      attribute fork but i_forkoff is zero.  This eliminates the ambiguity
      between i_forkoff and i_af.if_present, which should make it easier to
      understand the lifetime of attr forks.
      
      While we're at it, remove the if_present checks around calls to
      xfs_idestroy_fork and xfs_ifork_zap_attr since they can both handle attr
      forks that have already been torn down.
      Signed-off-by: NDarrick J. Wong <djwong@kernel.org>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      e45d7cb2
    • D
      xfs: make inode attribute forks a permanent part of struct xfs_inode · 2ed5b09b
      Darrick J. Wong 提交于
      Syzkaller reported a UAF bug a while back:
      
      ==================================================================
      BUG: KASAN: use-after-free in xfs_ilock_attr_map_shared+0xe3/0xf6 fs/xfs/xfs_inode.c:127
      Read of size 4 at addr ffff88802cec919c by task syz-executor262/2958
      
      CPU: 2 PID: 2958 Comm: syz-executor262 Not tainted
      5.15.0-0.30.3-20220406_1406 #3
      Hardware name: Red Hat KVM, BIOS 1.13.0-2.module+el8.3.0+7860+a7792d29
      04/01/2014
      Call Trace:
       <TASK>
       __dump_stack lib/dump_stack.c:88 [inline]
       dump_stack_lvl+0x82/0xa9 lib/dump_stack.c:106
       print_address_description.constprop.9+0x21/0x2d5 mm/kasan/report.c:256
       __kasan_report mm/kasan/report.c:442 [inline]
       kasan_report.cold.14+0x7f/0x11b mm/kasan/report.c:459
       xfs_ilock_attr_map_shared+0xe3/0xf6 fs/xfs/xfs_inode.c:127
       xfs_attr_get+0x378/0x4c2 fs/xfs/libxfs/xfs_attr.c:159
       xfs_xattr_get+0xe3/0x150 fs/xfs/xfs_xattr.c:36
       __vfs_getxattr+0xdf/0x13d fs/xattr.c:399
       cap_inode_need_killpriv+0x41/0x5d security/commoncap.c:300
       security_inode_need_killpriv+0x4c/0x97 security/security.c:1408
       dentry_needs_remove_privs.part.28+0x21/0x63 fs/inode.c:1912
       dentry_needs_remove_privs+0x80/0x9e fs/inode.c:1908
       do_truncate+0xc3/0x1e0 fs/open.c:56
       handle_truncate fs/namei.c:3084 [inline]
       do_open fs/namei.c:3432 [inline]
       path_openat+0x30ab/0x396d fs/namei.c:3561
       do_filp_open+0x1c4/0x290 fs/namei.c:3588
       do_sys_openat2+0x60d/0x98c fs/open.c:1212
       do_sys_open+0xcf/0x13c fs/open.c:1228
       do_syscall_x64 arch/x86/entry/common.c:50 [inline]
       do_syscall_64+0x3a/0x7e arch/x86/entry/common.c:80
       entry_SYSCALL_64_after_hwframe+0x44/0x0
      RIP: 0033:0x7f7ef4bb753d
      Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48
      89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73
      01 c3 48 8b 0d 1b 79 2c 00 f7 d8 64 89 01 48
      RSP: 002b:00007f7ef52c2ed8 EFLAGS: 00000246 ORIG_RAX: 0000000000000055
      RAX: ffffffffffffffda RBX: 0000000000404148 RCX: 00007f7ef4bb753d
      RDX: 00007f7ef4bb753d RSI: 0000000000000000 RDI: 0000000020004fc0
      RBP: 0000000000404140 R08: 0000000000000000 R09: 0000000000000000
      R10: 0000000000000000 R11: 0000000000000246 R12: 0030656c69662f2e
      R13: 00007ffd794db37f R14: 00007ffd794db470 R15: 00007f7ef52c2fc0
       </TASK>
      
      Allocated by task 2953:
       kasan_save_stack+0x19/0x38 mm/kasan/common.c:38
       kasan_set_track mm/kasan/common.c:46 [inline]
       set_alloc_info mm/kasan/common.c:434 [inline]
       __kasan_slab_alloc+0x68/0x7c mm/kasan/common.c:467
       kasan_slab_alloc include/linux/kasan.h:254 [inline]
       slab_post_alloc_hook mm/slab.h:519 [inline]
       slab_alloc_node mm/slub.c:3213 [inline]
       slab_alloc mm/slub.c:3221 [inline]
       kmem_cache_alloc+0x11b/0x3eb mm/slub.c:3226
       kmem_cache_zalloc include/linux/slab.h:711 [inline]
       xfs_ifork_alloc+0x25/0xa2 fs/xfs/libxfs/xfs_inode_fork.c:287
       xfs_bmap_add_attrfork+0x3f2/0x9b1 fs/xfs/libxfs/xfs_bmap.c:1098
       xfs_attr_set+0xe38/0x12a7 fs/xfs/libxfs/xfs_attr.c:746
       xfs_xattr_set+0xeb/0x1a9 fs/xfs/xfs_xattr.c:59
       __vfs_setxattr+0x11b/0x177 fs/xattr.c:180
       __vfs_setxattr_noperm+0x128/0x5e0 fs/xattr.c:214
       __vfs_setxattr_locked+0x1d4/0x258 fs/xattr.c:275
       vfs_setxattr+0x154/0x33d fs/xattr.c:301
       setxattr+0x216/0x29f fs/xattr.c:575
       __do_sys_fsetxattr fs/xattr.c:632 [inline]
       __se_sys_fsetxattr fs/xattr.c:621 [inline]
       __x64_sys_fsetxattr+0x243/0x2fe fs/xattr.c:621
       do_syscall_x64 arch/x86/entry/common.c:50 [inline]
       do_syscall_64+0x3a/0x7e arch/x86/entry/common.c:80
       entry_SYSCALL_64_after_hwframe+0x44/0x0
      
      Freed by task 2949:
       kasan_save_stack+0x19/0x38 mm/kasan/common.c:38
       kasan_set_track+0x1c/0x21 mm/kasan/common.c:46
       kasan_set_free_info+0x20/0x30 mm/kasan/generic.c:360
       ____kasan_slab_free mm/kasan/common.c:366 [inline]
       ____kasan_slab_free mm/kasan/common.c:328 [inline]
       __kasan_slab_free+0xe2/0x10e mm/kasan/common.c:374
       kasan_slab_free include/linux/kasan.h:230 [inline]
       slab_free_hook mm/slub.c:1700 [inline]
       slab_free_freelist_hook mm/slub.c:1726 [inline]
       slab_free mm/slub.c:3492 [inline]
       kmem_cache_free+0xdc/0x3ce mm/slub.c:3508
       xfs_attr_fork_remove+0x8d/0x132 fs/xfs/libxfs/xfs_attr_leaf.c:773
       xfs_attr_sf_removename+0x5dd/0x6cb fs/xfs/libxfs/xfs_attr_leaf.c:822
       xfs_attr_remove_iter+0x68c/0x805 fs/xfs/libxfs/xfs_attr.c:1413
       xfs_attr_remove_args+0xb1/0x10d fs/xfs/libxfs/xfs_attr.c:684
       xfs_attr_set+0xf1e/0x12a7 fs/xfs/libxfs/xfs_attr.c:802
       xfs_xattr_set+0xeb/0x1a9 fs/xfs/xfs_xattr.c:59
       __vfs_removexattr+0x106/0x16a fs/xattr.c:468
       cap_inode_killpriv+0x24/0x47 security/commoncap.c:324
       security_inode_killpriv+0x54/0xa1 security/security.c:1414
       setattr_prepare+0x1a6/0x897 fs/attr.c:146
       xfs_vn_change_ok+0x111/0x15e fs/xfs/xfs_iops.c:682
       xfs_vn_setattr_size+0x5f/0x15a fs/xfs/xfs_iops.c:1065
       xfs_vn_setattr+0x125/0x2ad fs/xfs/xfs_iops.c:1093
       notify_change+0xae5/0x10a1 fs/attr.c:410
       do_truncate+0x134/0x1e0 fs/open.c:64
       handle_truncate fs/namei.c:3084 [inline]
       do_open fs/namei.c:3432 [inline]
       path_openat+0x30ab/0x396d fs/namei.c:3561
       do_filp_open+0x1c4/0x290 fs/namei.c:3588
       do_sys_openat2+0x60d/0x98c fs/open.c:1212
       do_sys_open+0xcf/0x13c fs/open.c:1228
       do_syscall_x64 arch/x86/entry/common.c:50 [inline]
       do_syscall_64+0x3a/0x7e arch/x86/entry/common.c:80
       entry_SYSCALL_64_after_hwframe+0x44/0x0
      
      The buggy address belongs to the object at ffff88802cec9188
       which belongs to the cache xfs_ifork of size 40
      The buggy address is located 20 bytes inside of
       40-byte region [ffff88802cec9188, ffff88802cec91b0)
      The buggy address belongs to the page:
      page:00000000c3af36a1 refcount:1 mapcount:0 mapping:0000000000000000
      index:0x0 pfn:0x2cec9
      flags: 0xfffffc0000200(slab|node=0|zone=1|lastcpupid=0x1fffff)
      raw: 000fffffc0000200 ffffea00009d2580 0000000600000006 ffff88801a9ffc80
      raw: 0000000000000000 0000000080490049 00000001ffffffff 0000000000000000
      page dumped because: kasan: bad access detected
      
      Memory state around the buggy address:
       ffff88802cec9080: fb fb fb fc fc fa fb fb fb fb fc fc fb fb fb fb
       ffff88802cec9100: fb fc fc fb fb fb fb fb fc fc fb fb fb fb fb fc
      >ffff88802cec9180: fc fa fb fb fb fb fc fc fa fb fb fb fb fc fc fb
                                  ^
       ffff88802cec9200: fb fb fb fb fc fc fb fb fb fb fb fc fc fb fb fb
       ffff88802cec9280: fb fb fc fc fa fb fb fb fb fc fc fa fb fb fb fb
      ==================================================================
      
      The root cause of this bug is the unlocked access to xfs_inode.i_afp
      from the getxattr code paths while trying to determine which ILOCK mode
      to use to stabilize the xattr data.  Unfortunately, the VFS does not
      acquire i_rwsem when vfs_getxattr (or listxattr) call into the
      filesystem, which means that getxattr can race with a removexattr that's
      tearing down the attr fork and crash:
      
      xfs_attr_set:                          xfs_attr_get:
      xfs_attr_fork_remove:                  xfs_ilock_attr_map_shared:
      
      xfs_idestroy_fork(ip->i_afp);
      kmem_cache_free(xfs_ifork_cache, ip->i_afp);
      
                                             if (ip->i_afp &&
      
      ip->i_afp = NULL;
      
                                                 xfs_need_iread_extents(ip->i_afp))
                                             <KABOOM>
      
      ip->i_forkoff = 0;
      
      Regrettably, the VFS is much more lax about i_rwsem and getxattr than
      is immediately obvious -- not only does it not guarantee that we hold
      i_rwsem, it actually doesn't guarantee that we *don't* hold it either.
      The getxattr system call won't acquire the lock before calling XFS, but
      the file capabilities code calls getxattr with and without i_rwsem held
      to determine if the "security.capabilities" xattr is set on the file.
      
      Fixing the VFS locking requires a treewide investigation into every code
      path that could touch an xattr and what i_rwsem state it expects or sets
      up.  That could take years or even prove impossible; fortunately, we
      can fix this UAF problem inside XFS.
      
      An earlier version of this patch used smp_wmb in xfs_attr_fork_remove to
      ensure that i_forkoff is always zeroed before i_afp is set to null and
      changed the read paths to use smp_rmb before accessing i_forkoff and
      i_afp, which avoided these UAF problems.  However, the patch author was
      too busy dealing with other problems in the meantime, and by the time he
      came back to this issue, the situation had changed a bit.
      
      On a modern system with selinux, each inode will always have at least
      one xattr for the selinux label, so it doesn't make much sense to keep
      incurring the extra pointer dereference.  Furthermore, Allison's
      upcoming parent pointer patchset will also cause nearly every inode in
      the filesystem to have extended attributes.  Therefore, make the inode
      attribute fork structure part of struct xfs_inode, at a cost of 40 more
      bytes.
      
      This patch adds a clunky if_present field where necessary to maintain
      the existing logic of xattr fork null pointer testing in the existing
      codebase.  The next patch switches the logic over to XFS_IFORK_Q and it
      all goes away.
      Signed-off-by: NDarrick J. Wong <djwong@kernel.org>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      2ed5b09b
    • D
      xfs: convert XFS_IFORK_PTR to a static inline helper · 732436ef
      Darrick J. Wong 提交于
      We're about to make this logic do a bit more, so convert the macro to a
      static inline function for better typechecking and fewer shouty macros.
      No functional changes here.
      Signed-off-by: NDarrick J. Wong <djwong@kernel.org>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      732436ef
  3. 07 7月, 2022 2 次提交
  4. 29 4月, 2022 1 次提交
    • D
      xfs: stop artificially limiting the length of bunmap calls · 4ed6435c
      Darrick J. Wong 提交于
      In commit e1a4e37c, we clamped the length of bunmapi calls on the
      data forks of shared files to avoid two failure scenarios: one where the
      extent being unmapped is so sparsely shared that we exceed the
      transaction reservation with the sheer number of refcount btree updates
      and EFI intent items; and the other where we attach so many deferred
      updates to the transaction that we pin the log tail and later the log
      head meets the tail, causing the log to livelock.
      
      We avoid triggering the first problem by tracking the number of ops in
      the refcount btree cursor and forcing a requeue of the refcount intent
      item any time we think that we might be close to overflowing.  This has
      been baked into XFS since before the original e1a4 patch.
      
      A recent patchset fixed the second problem by changing the deferred ops
      code to finish all the work items created by each round of trying to
      complete a refcount intent item, which eliminates the long chains of
      deferred items (27dad); and causing long-running transactions to relog
      their intent log items when space in the log gets low (74f4d).
      
      Because this clamp affects /any/ unmapping request regardless of the
      sharing factors of the component blocks, it degrades the performance of
      all large unmapping requests -- whereas with an unshared file we can
      unmap millions of blocks in one go, shared files are limited to
      unmapping a few thousand blocks at a time, which causes the upper level
      code to spin in a bunmapi loop even if it wasn't needed.
      
      This also eliminates one more place where log recovery behavior can
      differ from online behavior, because bunmapi operations no longer need
      to requeue.  The fstest generic/447 was created to test the old fix, and
      it still passes with this applied.
      
      Partial-revert-of: e1a4e37c ("xfs: try to avoid blowing out the transaction reservation when bunmaping a shared extent")
      Depends: 27dada07 ("xfs: change the order in which child and parent defer ops ar finished")
      Depends: 74f4d6a1 ("xfs: only relog deferred intent items if free space in the log gets low")
      Signed-off-by: NDarrick J. Wong <djwong@kernel.org>
      Reviewed-by: NDave Chinner <dchinner@redhat.com>
      Reviewed-by: NChristoph Hellwig <hch@lst.de>
      4ed6435c
  5. 21 4月, 2022 2 次提交
  6. 13 4月, 2022 2 次提交
  7. 11 4月, 2022 6 次提交
  8. 05 12月, 2021 1 次提交
  9. 23 10月, 2021 4 次提交
  10. 20 10月, 2021 3 次提交
  11. 15 10月, 2021 1 次提交
  12. 20 8月, 2021 5 次提交
  13. 03 6月, 2021 1 次提交
  14. 02 6月, 2021 1 次提交
  15. 27 5月, 2021 2 次提交
    • D
      xfs: bunmapi has unnecessary AG lock ordering issues · 0fe0bbe0
      Dave Chinner 提交于
      large directory block size operations are assert failing because
      xfs_bunmapi() is not completely removing fragmented directory blocks
      like so:
      
      XFS: Assertion failed: done, file: fs/xfs/libxfs/xfs_dir2.c, line: 677
      ....
      Call Trace:
       xfs_dir2_shrink_inode+0x1a8/0x210
       xfs_dir2_block_to_sf+0x2ae/0x410
       xfs_dir2_block_removename+0x21a/0x280
       xfs_dir_removename+0x195/0x1d0
       xfs_rename+0xb79/0xc50
       ? avc_has_perm+0x8d/0x1a0
       ? avc_has_perm_noaudit+0x9a/0x120
       xfs_vn_rename+0xdb/0x150
       vfs_rename+0x719/0xb50
       ? __lookup_hash+0x6a/0xa0
       do_renameat2+0x413/0x5e0
       __x64_sys_rename+0x45/0x50
       do_syscall_64+0x3a/0x70
       entry_SYSCALL_64_after_hwframe+0x44/0xae
      
      We are aborting the bunmapi() pass because of this specific chunk of
      code:
      
                      /*
                       * Make sure we don't touch multiple AGF headers out of order
                       * in a single transaction, as that could cause AB-BA deadlocks.
                       */
                      if (!wasdel && !isrt) {
                              agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
                              if (prev_agno != NULLAGNUMBER && prev_agno > agno)
                                      break;
                              prev_agno = agno;
                      }
      
      This is designed to prevent deadlocks in AGF locking when freeing
      multiple extents by ensuring that we only ever lock in increasing
      AG number order. Unfortunately, this also violates the "bunmapi will
      always succeed" semantic that some high level callers depend on,
      such as xfs_dir2_shrink_inode(), xfs_da_shrink_inode() and
      xfs_inactive_symlink_rmt().
      
      This AG lock ordering was introduced back in 2017 to fix deadlocks
      triggered by generic/299 as reported here:
      
      https://lore.kernel.org/linux-xfs/800468eb-3ded-9166-20a4-047de8018582@gmail.com/
      
      This codebase is old enough that it was before we were defering all
      AG based extent freeing from within xfs_bunmapi(). THat is, we never
      actually lock AGs in xfs_bunmapi() any more - every non-rt based
      extent free is added to the defer ops list, as is all BMBT block
      freeing. And RT extents are not RT based, so there's no lock
      ordering issues associated with them.
      
      Hence this AGF lock ordering code is both broken and dead. Let's
      just remove it so that the large directory block code works reliably
      again.
      
      Tested against xfs/538 and generic/299 which is the original test
      that exposed the deadlocks that this code fixed.
      
      Fixes: 5b094d6d ("xfs: fix multi-AG deadlock in xfs_bunmapi")
      Signed-off-by: NDave Chinner <dchinner@redhat.com>
      Reviewed-by: NDarrick J. Wong <djwong@kernel.org>
      Signed-off-by: NDarrick J. Wong <djwong@kernel.org>
      0fe0bbe0
    • D
      xfs: btree format inode forks can have zero extents · 991c2c59
      Dave Chinner 提交于
      xfs/538 is assert failing with this trace when testing with
      directory block sizes of 64kB:
      
      XFS: Assertion failed: !xfs_need_iread_extents(ifp), file: fs/xfs/libxfs/xfs_bmap.c, line: 608
      ....
      Call Trace:
       xfs_bmap_btree_to_extents+0x2a9/0x470
       ? kmem_cache_alloc+0xe7/0x220
       __xfs_bunmapi+0x4ca/0xdf0
       xfs_bunmapi+0x1a/0x30
       xfs_dir2_shrink_inode+0x71/0x210
       xfs_dir2_block_to_sf+0x2ae/0x410
       xfs_dir2_block_removename+0x21a/0x280
       xfs_dir_removename+0x195/0x1d0
       xfs_remove+0x244/0x460
       xfs_vn_unlink+0x53/0xa0
       ? selinux_inode_unlink+0x13/0x20
       vfs_unlink+0x117/0x220
       do_unlinkat+0x1a2/0x2d0
       __x64_sys_unlink+0x42/0x60
       do_syscall_64+0x3a/0x70
       entry_SYSCALL_64_after_hwframe+0x44/0xae
      
      This is a check to ensure that the extents have been read into
      memory before we are doing a ifork btree manipulation. This assert
      is bogus in the above case.
      
      We have a fragmented directory block that has more extents in it
      than can fit in extent format, so the inode data fork is in btree
      format. xfs_dir2_shrink_inode() asks to remove all remaining 16
      filesystem blocks from the inode so it can convert to short form,
      and __xfs_bunmapi() removes all the extents. We now have a data fork
      in btree format but have zero extents in the fork. This incorrectly
      trips the xfs_need_iread_extents() assert because it assumes that an
      empty extent btree means the extent tree has not been read into
      memory yet. This is clearly not the case with xfs_bunmapi(), as it
      has an explicit call to xfs_iread_extents() in it to pull the
      extents into memory before it starts unmapping.
      
      Also, the assert directly after this bogus one is:
      
      	ASSERT(ifp->if_format == XFS_DINODE_FMT_BTREE);
      
      Which covers the context in which it is legal to call
      xfs_bmap_btree_to_extents just fine. Hence we should just remove the
      bogus assert as it is clearly wrong and causes a regression.
      
      The returns the test behaviour to the pre-existing assert failure in
      xfs_dir2_shrink_inode() that indicates xfs_bunmapi() has failed to
      remove all the extents in the range it was asked to unmap.
      Signed-off-by: NDave Chinner <dchinner@redhat.com>
      Reviewed-by: NDarrick J. Wong <djwong@kernel.org>
      Signed-off-by: NDarrick J. Wong <djwong@kernel.org>
      991c2c59
  16. 16 4月, 2021 4 次提交