On 2016/02/23 2:52, fdmanana@xxxxxxxxxx wrote:
From: Filipe Manana <fdmanana@xxxxxxxx>
In the listxattrs handler, we were not listing all the xattrs that are
packed in the same btree item, which happens when multiple xattrs have
a name that when crc32c hashed produce the same checksum value.
Fix this by processing them all.
The following test case for xfstests reproduces the issue:
seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
tmp=/tmp/$$
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
_cleanup()
{
cd /
rm -f $tmp.*
}
# get standard environment, filters and checks
. ./common/rc
. ./common/filter
. ./common/attr
# real QA test starts here
_supported_fs generic
_supported_os Linux
_require_scratch
_require_attrs
rm -f $seqres.full
_scratch_mkfs >>$seqres.full 2>&1
_scratch_mount
# Create our test file with a few xattrs. The first 3 xattrs have a name
# that when given as input to a crc32c function result in the same checksum.
# This made btrfs list only one of the xattrs through listxattrs system call
# (because it packs xattrs with the same name checksum into the same btree
# item).
touch $SCRATCH_MNT/testfile
$SETFATTR_PROG -n user.foobar -v 123 $SCRATCH_MNT/testfile
$SETFATTR_PROG -n user.WvG1c1Td -v qwerty $SCRATCH_MNT/testfile
$SETFATTR_PROG -n user.J3__T_Km3dVsW_ -v hello $SCRATCH_MNT/testfile
$SETFATTR_PROG -n user.something -v pizza $SCRATCH_MNT/testfile
$SETFATTR_PROG -n user.ping -v pong $SCRATCH_MNT/testfile
# Now call getfattr with --dump, which calls the listxattrs system call.
# It should list all the xattrs we have set before.
$GETFATTR_PROG --absolute-names --dump $SCRATCH_MNT/testfile | _filter_scratch
status=0
exit
Tested-by: Satoru Takeuchi <takeuchi_satoru@xxxxxxxxxxxxxx>
* chris/integration-4.6(HEAD is 790dd8b)
=======================================
# ./check generic/337
FSTYP -- btrfs
PLATFORM -- Linux/x86_64 fedora23 4.2.7-300.fc23.x86_64
MKFS_OPTIONS -- /dev/vdc
MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/vdc /scratch_mnt
generic/337 - output mismatch (see /root/xfstests/results//generic/337.out.bad)
--- tests/generic/337.out 2016-02-24 07:26:33.000000000 +0900
+++ /root/xfstests/results//generic/337.out.bad 2016-02-24 07:43:02.471000000 +0900
@@ -1,7 +1,5 @@
QA output created by 337
# file: SCRATCH_MNT/testfile
-user.J3__T_Km3dVsW_="hello"
-user.WvG1c1Td="qwerty"
user.foobar="123"
user.ping="pong"
user.something="pizza"
...
(Run 'diff -u tests/generic/337.out /root/xfstests/results//generic/337.out.bad' to see the entire diff)
Ran: generic/337
Failures: generic/337
Failed 1 of 1 tests
================================================================
* the above source + your patch
============================================================
# ./check generic/337
FSTYP -- btrfs
PLATFORM -- Linux/x86_64 fedora23 4.5.0-rc3-ktest+
MKFS_OPTIONS -- /dev/vdc
MOUNT_OPTIONS -- /dev/vdc /scratch_mnt
generic/337 0s
Ran: generic/337
Passed all 1 tests
============================================================
Thanks,
Satoru
Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
---
fs/btrfs/xattr.c | 62 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 23 deletions(-)
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index f2a20d5..caf643d 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -260,16 +260,12 @@ out:
ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
{
- struct btrfs_key key, found_key;
+ struct btrfs_key key;
struct inode *inode = d_inode(dentry);
struct btrfs_root *root = BTRFS_I(inode)->root;
struct btrfs_path *path;
- struct extent_buffer *leaf;
- struct btrfs_dir_item *di;
- int ret = 0, slot;
+ int ret = 0;
size_t total_size = 0, size_left = size;
- unsigned long name_ptr;
- size_t name_len;
/*
* ok we want all objects associated with this id.
@@ -291,6 +287,13 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
goto err;
while (1) {
+ struct extent_buffer *leaf;
+ int slot;
+ struct btrfs_dir_item *di;
+ struct btrfs_key found_key;
+ u32 item_size;
+ u32 cur;
+
leaf = path->nodes[0];
slot = path->slots[0];
@@ -319,28 +322,41 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
goto next;
di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
- if (verify_dir_item(root, leaf, di))
- goto next;
-
- name_len = btrfs_dir_name_len(leaf, di);
- total_size += name_len + 1;
+ item_size = btrfs_item_size_nr(leaf, slot);
+ cur = 0;
+ while (cur < item_size) {
+ u16 name_len = btrfs_dir_name_len(leaf, di);
+ u16 data_len = btrfs_dir_data_len(leaf, di);
+ u32 this_len = sizeof(*di) + name_len + data_len;
+ unsigned long name_ptr = (unsigned long)(di + 1);
+
+ if (verify_dir_item(root, leaf, di)) {
+ ret = -EIO;
+ goto err;
+ }
- /* we are just looking for how big our buffer needs to be */
- if (!size)
- goto next;
+ total_size += name_len + 1;
+ /*
+ * We are just looking for how big our buffer needs to
+ * be.
+ */
+ if (!size)
+ goto next;
- if (!buffer || (name_len + 1) > size_left) {
- ret = -ERANGE;
- goto err;
- }
+ if (!buffer || (name_len + 1) > size_left) {
+ ret = -ERANGE;
+ goto err;
+ }
- name_ptr = (unsigned long)(di + 1);
- read_extent_buffer(leaf, buffer, name_ptr, name_len);
- buffer[name_len] = '\0';
+ read_extent_buffer(leaf, buffer, name_ptr, name_len);
+ buffer[name_len] = '\0';
- size_left -= name_len + 1;
- buffer += name_len + 1;
+ size_left -= name_len + 1;
+ buffer += name_len + 1;
next:
+ cur += this_len;
+ di = (struct btrfs_dir_item *)((char *)di + this_len);
+ }
path->slots[0]++;
}
ret = total_size;
--
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
