Skip to content

Commit

Permalink
xfs: don't leak recovered attri intent items
Browse files Browse the repository at this point in the history
If recovery finds an xattr log intent item calling for the removal of an
attribute and the file doesn't even have an attr fork, we know that the
removal is trivially complete.  However, we can't just exit the recovery
function without doing something about the recovered log intent item --
it's still on the AIL, and not logging an attrd item means it stays
there forever.

This has likely not been seen in practice because few people use LARP
and the runtime code won't log the attri for a no-attrfork removexattr
operation.  But let's fix this anyway.

Also we shouldn't really be testing the attr fork presence until we've
taken the ILOCK, though this doesn't matter much in recovery, which is
single threaded.

Fixes: fdaf1bb ("xfs: ATTR_REPLACE algorithm with LARP enabled needs rework")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
  • Loading branch information
Darrick J. Wong committed Dec 7, 2023
1 parent 33cc938 commit 07bcbdf
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions fs/xfs/xfs_attr_item.c
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,13 @@ xfs_xattri_finish_update(
goto out;
}

/* If an attr removal is trivially complete, we're done. */
if (attr->xattri_op_flags == XFS_ATTRI_OP_FLAGS_REMOVE &&
!xfs_inode_hasattr(args->dp)) {
error = 0;
goto out;
}

error = xfs_attr_set_iter(attr);
if (!error && attr->xattri_dela_state != XFS_DAS_DONE)
error = -EAGAIN;
Expand Down Expand Up @@ -608,8 +615,6 @@ xfs_attri_item_recover(
attr->xattri_dela_state = xfs_attr_init_add_state(args);
break;
case XFS_ATTRI_OP_FLAGS_REMOVE:
if (!xfs_inode_hasattr(args->dp))
goto out;
attr->xattri_dela_state = xfs_attr_init_remove_state(args);
break;
default:
Expand Down

0 comments on commit 07bcbdf

Please sign in to comment.