On Fri, Nov 7, 2014 at 3:39 PM, Filipe Manana <fdmanana@xxxxxxxx> wrote:
Replacing a xattr consists of doing a lookup for its existing value,
delete
the current value from the respective leaf, release the search path
and then
finally insert the new value. This leaves a time window where readers
(getxattr,
listxattrs) won't see any value for the xattr. Xattrs are used to
store ACLs,
so this has security implications.
This change also fixes 2 other existing issues which were:
*) Deleting the old xattr value without verifying first if the new
xattr will
fit in the existing leaf item (in case multiple xattrs are packed
in the
same item due to name hash collision);
*) Returning -EEXIST when the flag XATTR_CREATE is given and the
xattr doesn't
exist but we have have an existing item that packs muliple xattrs
with
the same name hash as the input xattr. In this case we should
return ENOSPC.
A test case for xfstests follows soon.
Thanks to Alexandre Oliva for reporting the non-atomicity of the
xattr replace
implementation.
Thanks Filipe, one problem:
+
+ if (size > old_data_len) {
+ if (btrfs_leaf_free_space(root, leaf) <
+ (size - old_data_len)) {
+ ret = -ENOSPC;
+ goto out;
+ }
}
This means replacing an xattr item can fail if the leaf doesn't happen
to have free space. btrfs_search_slot already has code meant to extend
the item, even if the item you're searching for is already found. (see
the call to split_leaf with extend == 1).
So what you want to do is find out the current size of the item, and
call btrfs search_slot again with that size + the extra needed for the
new name. From there you should be able to call btrfs_extend_item like
you currently are.
-chris
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
