btrfs-progs mkfs.btrfs tool giving error "unable to add xattr items for" if only some files in root directory specified in --rootdir DIR has capabiliites set.

[jiteshchal]

The mkfs.btrfs tool ( v4.20 and previous version also) seems to be giving error when all the files in the root directory specified (--rootdir DIR) is not having capabilities set and only few files have capabilities set.
The mkfs.btrfs tool is not giving error if all the files in the root directory specified (--rootdir DIR) does not have capabilities set, only if some of the files in root directory has capabilities set it is giving error for remaining files which does not capabilities set and exiting.
The following are the steps tried when the issue is observed.

[root@localhost btrfs-progs-v4.20]# uname -a
Linux localhost.localdomain 4.13.16-100.fc25.x86_64 #1 SMP Mon Nov 27 19:52:46 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

In the example below for the root directory specified (romfs) only capability is set for ping utility and for busybox utlility capability is not set. When we run the mkfs.btrfs tool we are getting the error "unable to add xattr items for busybox" and tool exits as mentioned in following logs.
[root@localhost btrfs-progs-v4.20]# getcap ./romfs/bin/busybox
[root@localhost btrfs-progs-v4.20]# getcap ./romfs/bin/ping
./romfs/bin/ping = cap_net_raw+ei
[root@localhost btrfs-progs-v4.20]# ./mkfs.btrfs --shrink -f -m single -d single -r ./romfs ./rootfs.btrfs
btrfs-progs v4.20
See http://btrfs.wiki.kernel.org for more information.

ERROR: getting a xattr value failed for busybox attr security.capability: No data available
ERROR: unable to add xattr items for busybox: -1
ERROR: unable to traverse directory ./romfs: 1
ERROR: error while filling filesystem: 1
[root@localhost btrfs-progs-v4.20]#

In the example below for the root directory specified (romfs_new) for both ping utility and for busybox utlility capability is not set. When we run the mkfs.btrfs tool we are not getting any error and btrfs image is getting generated as mentioned in following logs.
[root@localhost btrfs-progs-v4.20]# getcap ./romfs_new/bin/busybox
[root@localhost btrfs-progs-v4.20]# getcap ./romfs_new/bin/ping
[root@localhost btrfs-progs-v4.20]#
[root@localhost btrfs-progs-v4.20]# ./mkfs.btrfs --shrink -f -m single -d single -r ./romfs_new ./rootfs.btrfs
btrfs-progs v4.20
See http://btrfs.wiki.kernel.org for more information.

ERROR: superblock magic doesn't match
Making image is completed.
Label: (null)
UUID: 6d03c8cf-7908-4f3d-9166-830896a54b17
Node size: 16384
Sector size: 4096
Filesystem size: 245.00MiB
Block group profiles:
Data: single 224.00MiB
Metadata: single 16.00MiB
System: single 4.00MiB
SSD detected: no
Incompat features: extref, skinny-metadata
Number of devices: 1
Devices:
ID SIZE PATH
1 245.00MiB ./rootfs.btrfs

[root@localhost btrfs-progs-v4.20]#

Same behavior is seen with older versions of btrfs-progs also like v4.17.1

Please suggest if there any patches available for fixing this issue.

We tried adding a patch to the tool to ignore the warning if capabilities are not set for some files in root directory specified and continue with building and creating the btrfs root file system image with the files which has capabilities set.
The change done for this patch is following in the file mkfs/rootdir.c. With the change the btrfs image is getting build with retaining the capabilities for the files set in root directory specified.

diff -Naur btrfs-progs-4.17.1/mkfs/rootdir.c btrfs-progs-4.17.1_new/mkfs/rootdir.c
--- btrfs-progs-4.17.1/mkfs/rootdir.c 2019-07-26 11:34:44.473611317 +0530
+++ btrfs-progs-4.17.1_new/mkfs/rootdir.c 2019-07-26 11:37:19.666351536 +0530
@@ -567,8 +567,8 @@
if (ret) { error("unable to add xattr items for %s: %d",
cur_file->d_name, ret);- if (ret != -ENOTSUP)
- goto fail; + //if (ret != -ENOTSUP)
+ // goto fail; }

                    if (S_ISDIR(st.st_mode)) {

Kindly please review the above patch and also please suggest if we need to make any other changes.

[CyberShadow]

I couldn't reproduce this problem.

$ mkdir romfs romfs/bin
$ touch romfs/bin/{busybox,ping}
$ sudo setcap cap_net_raw+ei romfs/bin/ping 
$ getcap romfs/bin/ping
romfs/bin/ping = cap_net_raw+ei
$ ./mkfs.btrfs --shrink -f -m single -d single -r ./romfs ./rootfs.btrfs
btrfs-progs v5.2.1 
See http://btrfs.wiki.kernel.org for more information.

Making image is completed.
Label:              (null)
UUID:               d5a629ec-3585-40ea-b4e7-601f787e6e0a
Node size:          16384
Sector size:        4096
Filesystem size:    14.00MiB
Block group profiles:
  Data:             single            4.50MiB
  Metadata:         single            4.50MiB
  System:           single            4.00MiB
SSD detected:       no
Incompat features:  extref, skinny-metadata
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1    14.00MiB  ./rootfs.btrfs
$ uname -a
Linux home.thecybershadow.net 5.2.6-arch1-1-ARCH #1 SMP PREEMPT Sun Aug 4 14:58:49 UTC 2019 x86_64 GNU/Linux
$ git rev-parse HEAD
9a85732d8beaae4b80cab98bb3355660389c1d36

Perhaps the old version the kernel and of btrfs-progs is the issue, or (my interpretation of) the reproduction steps you provided are incomplete.

[jiteshchal]

Hi,

Thanks for reviewing the error.
I was able to reproduce the issue with the latest version of tool v5.2.1 also by following the same steps as above. Attaching the steps followed.
[root@localhost btrfs-progs-v5.2.1]# mkdir romfs romfs/bin
[root@localhost btrfs-progs-v5.2.1]# touch romfs/bin/{busybox,ping}
[root@localhost btrfs-progs-v5.2.1]# sudo setcap cap_net_raw+ei romfs/bin/ping
[root@localhost btrfs-progs-v5.2.1]# getcap romfs/bin/ping
romfs/bin/ping = cap_net_raw+ei
[root@localhost btrfs-progs-v5.2.1]# ./mkfs.btrfs --shrink -f -m single -d single -r ./romfs ./rootfs.btrfs
btrfs-progs v5.2.1
See http://btrfs.wiki.kernel.org for more information.

ERROR: getting a xattr value failed for busybox attr security.capability: No data available
ERROR: unable to add xattr items for busybox: -1
ERROR: unable to traverse directory ./romfs: 1
ERROR: error while filling filesystem: 1

I was able to reproduce the issue with older version of btrfs-progs tool also like btrfs-progs-v4.17.1 and btrfs-progs-v4.20.

The kernel version i am using is 4.13.16-100.fc25.x86_64.
[root@localhost btrfs-progs-v4.20]# uname -a
Linux localhost.localdomain 4.13.16-100.fc25.x86_64 #1 SMP Mon Nov 27 19:52:46 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

With Thanks and Regards
Jitesh

[CyberShadow]

I tried performing these steps on a variety of machines (with various kernel and btrfs-progs versions) and host filesystems and still haven't been able to reproduce this problem.

However, I have a hunch at what might be the cause. Could you please test the following patch:

From 5897650ebeada2f67334d4cde62ea4c373e33847 Mon Sep 17 00:00:00 2001
From: Vladimir Panteleev <git@vladimir.panteleev.md>
Date: Thu, 22 Aug 2019 18:25:59 +0000
Subject: [PATCH] btrfs-progs: mkfs: fix xattr enumeration

Use the return value of listxattr instead of tokenizing.

The end of the extended attribute list is indicated by the return
value, not an empty list item (two consecutive NULs). Using strtok
in this way thus sometimes caused add_xattr_item to reuse stack data
in xattr_list from the previous invocation, thus querying attributes
that are not actually in the file's xattr list.

Issue: #194
---
 mkfs/rootdir.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
index 51411e02..062d959c 100644
--- a/mkfs/rootdir.c
+++ b/mkfs/rootdir.c
@@ -227,11 +227,10 @@ static int add_xattr_item(struct btrfs_trans_handle *trans,
 {
 	int ret;
 	int cur_name_len;
+	int xattr_list_len;
 	char xattr_list[XATTR_LIST_MAX];
 	char *cur_name;
 	char cur_value[XATTR_SIZE_MAX];
-	char delimiter = '\0';
-	char *next_location = xattr_list;
 
 	ret = llistxattr(file_name, xattr_list, XATTR_LIST_MAX);
 	if (ret < 0) {
@@ -243,10 +242,10 @@ static int add_xattr_item(struct btrfs_trans_handle *trans,
 	if (ret == 0)
 		return ret;
 
-	cur_name = strtok(xattr_list, &delimiter);
-	while (cur_name != NULL) {
+	xattr_list_len = ret;
+	cur_name = xattr_list;
+	while (cur_name - xattr_list < xattr_list_len) {
 		cur_name_len = strlen(cur_name);
-		next_location += cur_name_len + 1;
 
 		ret = lgetxattr(file_name, cur_name, cur_value, XATTR_SIZE_MAX);
 		if (ret < 0) {
@@ -266,7 +265,7 @@ static int add_xattr_item(struct btrfs_trans_handle *trans,
 					file_name);
 		}
 
-		cur_name = strtok(next_location, &delimiter);
+		cur_name += cur_name_len + 1;
 	}
 
 	return ret;
-- 
2.22.0

Also, whether or not the patch works, it would help if you could please post the output of running the mkfs.btrfs command with strace.

[jiteshchal]

Hi,

Thanks for reviewing and providing the patch. I applied the above patch and tried to build the btrfs root file system image and the issue is not getting reproduced. I am able to build the btrfs
file system image with the patch.

Also as requested please find attached the strace output for mkfs.btrfs when the issue was getting reproduced.

With Thanks and Regards
Jitesh
strace_output.txt

[CyberShadow]

Thank you! I confirmed from the strace output that my theory was correct. Relevant fragment:

lstat("ping", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
llistxattr("ping", "security.selinux\0security.capabi"..., 65536) = 37
lgetxattr("ping", "security.selinux", "unconfined_u:object_r:admin_home"..., 65536) = 38
lgetxattr("ping", "security.capability", "\1\0\0\2\0\0\0\0\0 \0\0\0\0\0\0\0\0\0", 65536) = 20
lstat("busybox", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
llistxattr("busybox", "security.selinux\0", 65536) = 17
lgetxattr("busybox", "security.selinux", "unconfined_u:object_r:admin_home"..., 65536) = 38
lgetxattr("busybox", "security.capability", 0x7fff220e27c0, 65536) = -1 ENODATA (No data available)

I've submitted the patch to the mailing list for inclusion.

[kdave]

Do you have a reproducer that we can add to our testsuite? The capabilities in send have been problematic (and we might still have some bugs there), so a regression test would be most welcome.

[marcosps]

@jiteshchal can we close this bug? This patch that fixes this issue was already merged.