btrfs: validate properties before setting them

We set the xattr and then attempt to apply the property. If the apply
fails we then attempt to delete the xattr to avoid an inconsistency.
However we don't verify if the deletion succeed, so if it fails we
leave an inconsistency between the state in the btree and the in-memory
inode.

Address this by validating first if we can apply the property, then set
the xattr, then apply the property, and this last step should not fail
since the validation succeeded before - assert that it does not fail but
leave code to attempt to delete the xattr if it happens, and then abort
the transaction only if the xattr delete failed.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index adc956432d2f1501d27ebeed3e7c437715361bf7..bb77d46376d4bb2c1033f478f1a5ee422375e253 100644 (file)
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -127,14 +127,24 @@ int btrfs_set_prop(struct btrfs_trans_handle *trans, struct btrfs_inode *inode,
                return ret;
        }
 
+       ret = handler->validate(inode, value, value_len);
+       if (ret)
+               return ret;
        ret = btrfs_setxattr(trans, &inode->vfs_inode, handler->xattr_name, value,
                             value_len, flags);
        if (ret)
                return ret;
        ret = handler->apply(inode, value, value_len);
-       if (ret) {
-               btrfs_setxattr(trans, &inode->vfs_inode, handler->xattr_name, NULL,
-                              0, flags);
+       /* We validated before, so it should not fail here. */
+       ASSERT(ret == 0);
+       if (unlikely(ret)) {
+               int ret2;
+
+               /* Try to delete xattr, if not possible abort transaction. */
+               ret2 = btrfs_setxattr(trans, &inode->vfs_inode, handler->xattr_name,
+                                     NULL, 0, flags);
+               if (unlikely(ret2))
+                       btrfs_abort_transaction(trans, ret2);
                return ret;
        }