On Wed, May 18, 2022 at 11:54:41AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > kmemleak reported that we lost an xfs_da_state while removing xattrs in > generic/020: > > unreferenced object 0xffff88801c0e4b40 (size 480): > comm "attr", pid 30515, jiffies 4294931061 (age 5.960s) > hex dump (first 32 bytes): > 78 bc 65 07 00 c9 ff ff 00 30 60 1c 80 88 ff ff x.e......0`..... > 02 00 00 00 00 00 00 00 80 18 83 4e 80 88 ff ff ...........N.... > backtrace: > [<ffffffffa023ef4a>] xfs_da_state_alloc+0x1a/0x30 [xfs] > [<ffffffffa021b6f3>] xfs_attr_node_hasname+0x23/0x90 [xfs] > [<ffffffffa021c6f1>] xfs_attr_set_iter+0x441/0xa30 [xfs] > [<ffffffffa02b5104>] xfs_xattri_finish_update+0x44/0x80 [xfs] > [<ffffffffa02b515e>] xfs_attr_finish_item+0x1e/0x40 [xfs] > [<ffffffffa0244744>] xfs_defer_finish_noroll+0x184/0x740 [xfs] > [<ffffffffa02a6473>] __xfs_trans_commit+0x153/0x3e0 [xfs] > [<ffffffffa021d149>] xfs_attr_set+0x469/0x7e0 [xfs] > [<ffffffffa02a78d9>] xfs_xattr_set+0x89/0xd0 [xfs] > [<ffffffff812e6512>] __vfs_removexattr+0x52/0x70 > [<ffffffff812e6a08>] __vfs_removexattr_locked+0xb8/0x150 > [<ffffffff812e6af6>] vfs_removexattr+0x56/0x100 > [<ffffffff812e6bf8>] removexattr+0x58/0x90 > [<ffffffff812e6cce>] path_removexattr+0x9e/0xc0 > [<ffffffff812e6d44>] __x64_sys_lremovexattr+0x14/0x20 > [<ffffffff81786b35>] do_syscall_64+0x35/0x80 > > I think this is a consequence of xfs_attr_node_removename_setup > attaching a new da(btree) state to xfs_attr_item and never freeing it. > I /think/ it's the case that the remove paths could detach the da state > earlier in the remove state machine since nothing else accesses the > state. However, let's future-proof the new xattr code by adding a > catch-all when we free the xfs_attr_item to make sure we never leak the > da state. > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> > Reviewed-by: Allison Henderson <allison.henderson@xxxxxxxxxx> > --- > fs/xfs/libxfs/xfs_attr.c | 22 ++++++++++++++-------- > fs/xfs/xfs_attr_item.c | 15 ++++++++++++--- > 2 files changed, 26 insertions(+), 11 deletions(-) Looks ok. Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> -- Dave Chinner david@xxxxxxxxxxxxx
