xfs: don't leak recovered attri intent items
authorDarrick J. Wong <djwong@kernel.org>
Thu, 23 Nov 2023 17:48:09 +0000 (09:48 -0800)
committerDarrick J. Wong <djwong@kernel.org>
Thu, 7 Dec 2023 02:45:14 +0000 (18:45 -0800)
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: fdaf1bb3cafc ("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>
fs/xfs/xfs_attr_item.c

index 36fe2abb16e6e35563a0f7d5ca8ed895c4ff2130..11e88a76a33c00a99ac107fa4b1e63d534268737 100644 (file)
@@ -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;
@@ -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: