* [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
@ 2025-06-23 6:38 Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex Song Liu
` (5 more replies)
0 siblings, 6 replies; 23+ messages in thread
From: Song Liu @ 2025-06-23 6:38 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-kernel, linux-security-module
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, amir73il, gregkh, tj,
daan.j.demeyer, Song Liu
Introduce a new kfunc bpf_cgroup_read_xattr, which can read xattr from
cgroupfs nodes. The primary users are LSMs, cgroup programs, and sched_ext.
---
Changes v2 => v3:
1. Make bpf_cgroup_read_xattr available to all program types.
2. Fix gcc build warning on the selftests.
3. Add "ifdef CONFIG_CGROUPS" around bpf_cgroup_read_xattr.
v2: https://lore.kernel.org/bpf/20250619220114.3956120-1-song@kernel.org/
Changes v1 => v2:
1. Replace 1/4 in v1 with Chritian's version (1/5 in v2).
2. Rename bpf_kernfs_read_xattr => bpf_cgroup_read_xattr, and limit access
to cgroup only.
3. Add 5/5, which makes bpf_cgroup_read_xattr available to cgroup and
struct_ops programs.
v1: https://lore.kernel.org/bpf/20250618233739.189106-1-song@kernel.org/
Christian Brauner (1):
kernfs: remove iattr_mutex
Song Liu (3):
bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node
bpf: Mark cgroup_subsys_state->cgroup RCU safe
selftests/bpf: Add tests for bpf_cgroup_read_xattr
fs/bpf_fs_kfuncs.c | 34 ++++
fs/kernfs/inode.c | 74 ++++----
kernel/bpf/helpers.c | 3 +
kernel/bpf/verifier.c | 5 +
.../testing/selftests/bpf/bpf_experimental.h | 3 +
.../selftests/bpf/prog_tests/cgroup_xattr.c | 145 ++++++++++++++++
.../selftests/bpf/progs/cgroup_read_xattr.c | 158 ++++++++++++++++++
.../selftests/bpf/progs/read_cgroupfs_xattr.c | 60 +++++++
8 files changed, 448 insertions(+), 34 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_xattr.c
create mode 100644 tools/testing/selftests/bpf/progs/cgroup_read_xattr.c
create mode 100644 tools/testing/selftests/bpf/progs/read_cgroupfs_xattr.c
--
2.47.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex
2025-06-23 6:38 [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Song Liu
@ 2025-06-23 6:38 ` Song Liu
2025-07-02 10:47 ` André Draszik
2025-06-23 6:38 ` [PATCH v3 bpf-next 2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node Song Liu
` (4 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Song Liu @ 2025-06-23 6:38 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-kernel, linux-security-module
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, amir73il, gregkh, tj,
daan.j.demeyer, Song Liu
From: Christian Brauner <brauner@kernel.org>
All allocations of struct kernfs_iattrs are serialized through a global
mutex. Simply do a racy allocation and let the first one win. I bet most
callers are under inode->i_rwsem anyway and it wouldn't be needed but
let's not require that.
Signed-off-by: Christian Brauner <brauner@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Song Liu <song@kernel.org>
---
fs/kernfs/inode.c | 74 +++++++++++++++++++++++++----------------------
1 file changed, 40 insertions(+), 34 deletions(-)
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index b83054da68b3..f4b73b9482b7 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -24,45 +24,46 @@ static const struct inode_operations kernfs_iops = {
.listxattr = kernfs_iop_listxattr,
};
-static struct kernfs_iattrs *__kernfs_iattrs(struct kernfs_node *kn, int alloc)
+static struct kernfs_iattrs *__kernfs_iattrs(struct kernfs_node *kn, bool alloc)
{
- static DEFINE_MUTEX(iattr_mutex);
- struct kernfs_iattrs *ret;
+ struct kernfs_iattrs *ret __free(kfree) = NULL;
+ struct kernfs_iattrs *attr;
- mutex_lock(&iattr_mutex);
+ attr = READ_ONCE(kn->iattr);
+ if (attr || !alloc)
+ return attr;
- if (kn->iattr || !alloc)
- goto out_unlock;
-
- kn->iattr = kmem_cache_zalloc(kernfs_iattrs_cache, GFP_KERNEL);
- if (!kn->iattr)
- goto out_unlock;
+ ret = kmem_cache_zalloc(kernfs_iattrs_cache, GFP_KERNEL);
+ if (!ret)
+ return NULL;
/* assign default attributes */
- kn->iattr->ia_uid = GLOBAL_ROOT_UID;
- kn->iattr->ia_gid = GLOBAL_ROOT_GID;
-
- ktime_get_real_ts64(&kn->iattr->ia_atime);
- kn->iattr->ia_mtime = kn->iattr->ia_atime;
- kn->iattr->ia_ctime = kn->iattr->ia_atime;
-
- simple_xattrs_init(&kn->iattr->xattrs);
- atomic_set(&kn->iattr->nr_user_xattrs, 0);
- atomic_set(&kn->iattr->user_xattr_size, 0);
-out_unlock:
- ret = kn->iattr;
- mutex_unlock(&iattr_mutex);
- return ret;
+ ret->ia_uid = GLOBAL_ROOT_UID;
+ ret->ia_gid = GLOBAL_ROOT_GID;
+
+ ktime_get_real_ts64(&ret->ia_atime);
+ ret->ia_mtime = ret->ia_atime;
+ ret->ia_ctime = ret->ia_atime;
+
+ simple_xattrs_init(&ret->xattrs);
+ atomic_set(&ret->nr_user_xattrs, 0);
+ atomic_set(&ret->user_xattr_size, 0);
+
+ /* If someone raced us, recognize it. */
+ if (!try_cmpxchg(&kn->iattr, &attr, ret))
+ return READ_ONCE(kn->iattr);
+
+ return no_free_ptr(ret);
}
static struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
{
- return __kernfs_iattrs(kn, 1);
+ return __kernfs_iattrs(kn, true);
}
static struct kernfs_iattrs *kernfs_iattrs_noalloc(struct kernfs_node *kn)
{
- return __kernfs_iattrs(kn, 0);
+ return __kernfs_iattrs(kn, false);
}
int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr)
@@ -141,9 +142,9 @@ ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
struct kernfs_node *kn = kernfs_dentry_node(dentry);
struct kernfs_iattrs *attrs;
- attrs = kernfs_iattrs(kn);
+ attrs = kernfs_iattrs_noalloc(kn);
if (!attrs)
- return -ENOMEM;
+ return -ENODATA;
return simple_xattr_list(d_inode(dentry), &attrs->xattrs, buf, size);
}
@@ -166,9 +167,10 @@ static inline void set_inode_attr(struct inode *inode,
static void kernfs_refresh_inode(struct kernfs_node *kn, struct inode *inode)
{
- struct kernfs_iattrs *attrs = kn->iattr;
+ struct kernfs_iattrs *attrs;
inode->i_mode = kn->mode;
+ attrs = kernfs_iattrs_noalloc(kn);
if (attrs)
/*
* kernfs_node has non-default attributes get them from
@@ -306,7 +308,9 @@ int kernfs_xattr_set(struct kernfs_node *kn, const char *name,
const void *value, size_t size, int flags)
{
struct simple_xattr *old_xattr;
- struct kernfs_iattrs *attrs = kernfs_iattrs(kn);
+ struct kernfs_iattrs *attrs;
+
+ attrs = kernfs_iattrs(kn);
if (!attrs)
return -ENOMEM;
@@ -345,8 +349,9 @@ static int kernfs_vfs_user_xattr_add(struct kernfs_node *kn,
struct simple_xattrs *xattrs,
const void *value, size_t size, int flags)
{
- atomic_t *sz = &kn->iattr->user_xattr_size;
- atomic_t *nr = &kn->iattr->nr_user_xattrs;
+ struct kernfs_iattrs *attr = kernfs_iattrs_noalloc(kn);
+ atomic_t *sz = &attr->user_xattr_size;
+ atomic_t *nr = &attr->nr_user_xattrs;
struct simple_xattr *old_xattr;
int ret;
@@ -384,8 +389,9 @@ static int kernfs_vfs_user_xattr_rm(struct kernfs_node *kn,
struct simple_xattrs *xattrs,
const void *value, size_t size, int flags)
{
- atomic_t *sz = &kn->iattr->user_xattr_size;
- atomic_t *nr = &kn->iattr->nr_user_xattrs;
+ struct kernfs_iattrs *attr = kernfs_iattrs(kn);
+ atomic_t *sz = &attr->user_xattr_size;
+ atomic_t *nr = &attr->nr_user_xattrs;
struct simple_xattr *old_xattr;
old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags);
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 bpf-next 2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node
2025-06-23 6:38 [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex Song Liu
@ 2025-06-23 6:38 ` Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe Song Liu
` (3 subsequent siblings)
5 siblings, 0 replies; 23+ messages in thread
From: Song Liu @ 2025-06-23 6:38 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-kernel, linux-security-module
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, amir73il, gregkh, tj,
daan.j.demeyer, Song Liu
BPF programs, such as LSM and sched_ext, would benefit from tags on
cgroups. One common practice to apply such tags is to set xattrs on
cgroupfs folders.
Introduce kfunc bpf_cgroup_read_xattr, which allows reading cgroup's
xattr.
Note that, we already have bpf_get_[file|dentry]_xattr. However, these
two APIs are not ideal for reading cgroupfs xattrs, because:
1) These two APIs only works in sleepable contexts;
2) There is no kfunc that matches current cgroup to cgroupfs dentry.
bpf_cgroup_read_xattr is generic and can be useful for many program
types. It is also safe, because it requires trusted or rcu protected
argument (KF_RCU). Therefore, we make it available to all program types.
Signed-off-by: Song Liu <song@kernel.org>
Acked-by: Tejun Heo <tj@kernel.org>
---
fs/bpf_fs_kfuncs.c | 34 ++++++++++++++++++++++++++++++++++
kernel/bpf/helpers.c | 3 +++
2 files changed, 37 insertions(+)
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 08412532db1b..1e36a12b88f7 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -9,6 +9,7 @@
#include <linux/fs.h>
#include <linux/fsnotify.h>
#include <linux/file.h>
+#include <linux/kernfs.h>
#include <linux/mm.h>
#include <linux/xattr.h>
@@ -322,6 +323,39 @@ __bpf_kfunc int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name_
return ret;
}
+#ifdef CONFIG_CGROUPS
+/**
+ * bpf_cgroup_read_xattr - read xattr of a cgroup's node in cgroupfs
+ * @cgroup: cgroup to get xattr from
+ * @name__str: name of the xattr
+ * @value_p: output buffer of the xattr value
+ *
+ * Get xattr *name__str* of *cgroup* and store the output in *value_ptr*.
+ *
+ * For security reasons, only *name__str* with prefix "user." is allowed.
+ *
+ * Return: length of the xattr value on success, a negative value on error.
+ */
+__bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__str,
+ struct bpf_dynptr *value_p)
+{
+ struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p;
+ u32 value_len;
+ void *value;
+
+ /* Only allow reading "user.*" xattrs */
+ if (strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
+ return -EPERM;
+
+ value_len = __bpf_dynptr_size(value_ptr);
+ value = __bpf_dynptr_data_rw(value_ptr, value_len);
+ if (!value)
+ return -EINVAL;
+
+ return kernfs_xattr_get(cgroup->kn, name__str, value, value_len);
+}
+#endif /* CONFIG_CGROUPS */
+
__bpf_kfunc_end_defs();
BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b71e428ad936..9ff1b4090289 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3397,6 +3397,9 @@ BTF_ID_FLAGS(func, bpf_iter_dmabuf_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPAB
BTF_ID_FLAGS(func, bpf_iter_dmabuf_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
#endif
BTF_ID_FLAGS(func, __bpf_trap)
+#ifdef CONFIG_CGROUPS
+BTF_ID_FLAGS(func, bpf_cgroup_read_xattr, KF_RCU)
+#endif
BTF_KFUNCS_END(common_btf_ids)
static const struct btf_kfunc_id_set common_kfunc_set = {
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 bpf-next 3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe
2025-06-23 6:38 [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node Song Liu
@ 2025-06-23 6:38 ` Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr Song Liu
` (2 subsequent siblings)
5 siblings, 0 replies; 23+ messages in thread
From: Song Liu @ 2025-06-23 6:38 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-kernel, linux-security-module
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, amir73il, gregkh, tj,
daan.j.demeyer, Song Liu
Mark struct cgroup_subsys_state->cgroup as safe under RCU read lock. This
will enable accessing css->cgroup from a bpf css iterator.
Signed-off-by: Song Liu <song@kernel.org>
---
kernel/bpf/verifier.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 279a64933262..e2f53dc8766a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7058,6 +7058,10 @@ BTF_TYPE_SAFE_RCU(struct css_set) {
struct cgroup *dfl_cgrp;
};
+BTF_TYPE_SAFE_RCU(struct cgroup_subsys_state) {
+ struct cgroup *cgroup;
+};
+
/* RCU trusted: these fields are trusted in RCU CS and can be NULL */
BTF_TYPE_SAFE_RCU_OR_NULL(struct mm_struct) {
struct file __rcu *exe_file;
@@ -7108,6 +7112,7 @@ static bool type_is_rcu(struct bpf_verifier_env *env,
BTF_TYPE_EMIT(BTF_TYPE_SAFE_RCU(struct task_struct));
BTF_TYPE_EMIT(BTF_TYPE_SAFE_RCU(struct cgroup));
BTF_TYPE_EMIT(BTF_TYPE_SAFE_RCU(struct css_set));
+ BTF_TYPE_EMIT(BTF_TYPE_SAFE_RCU(struct cgroup_subsys_state));
return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id, "__safe_rcu");
}
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 bpf-next 4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr
2025-06-23 6:38 [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Song Liu
` (2 preceding siblings ...)
2025-06-23 6:38 ` [PATCH v3 bpf-next 3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe Song Liu
@ 2025-06-23 6:38 ` Song Liu
2025-06-23 11:03 ` [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Christian Brauner
2025-06-27 2:20 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 23+ messages in thread
From: Song Liu @ 2025-06-23 6:38 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-kernel, linux-security-module
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, amir73il, gregkh, tj,
daan.j.demeyer, Song Liu
Add tests for different scenarios with bpf_cgroup_read_xattr:
1. Read cgroup xattr from bpf_cgroup_from_id;
2. Read cgroup xattr from bpf_cgroup_ancestor;
3. Read cgroup xattr from css_iter;
4. Use bpf_cgroup_read_xattr in LSM hook security_socket_connect.
5. Use bpf_cgroup_read_xattr in cgroup program.
Signed-off-by: Song Liu <song@kernel.org>
---
.../testing/selftests/bpf/bpf_experimental.h | 3 +
.../selftests/bpf/prog_tests/cgroup_xattr.c | 145 ++++++++++++++++
.../selftests/bpf/progs/cgroup_read_xattr.c | 158 ++++++++++++++++++
.../selftests/bpf/progs/read_cgroupfs_xattr.c | 60 +++++++
4 files changed, 366 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_xattr.c
create mode 100644 tools/testing/selftests/bpf/progs/cgroup_read_xattr.c
create mode 100644 tools/testing/selftests/bpf/progs/read_cgroupfs_xattr.c
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 5e512a1d09d1..da7e230f2781 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -596,4 +596,7 @@ extern int bpf_iter_dmabuf_new(struct bpf_iter_dmabuf *it) __weak __ksym;
extern struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it) __weak __ksym;
extern void bpf_iter_dmabuf_destroy(struct bpf_iter_dmabuf *it) __weak __ksym;
+extern int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__str,
+ struct bpf_dynptr *value_p) __weak __ksym;
+
#endif
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_xattr.c b/tools/testing/selftests/bpf/prog_tests/cgroup_xattr.c
new file mode 100644
index 000000000000..87978a0f7eb7
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_xattr.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/socket.h>
+#include <sys/xattr.h>
+
+#include <test_progs.h>
+
+#include "read_cgroupfs_xattr.skel.h"
+#include "cgroup_read_xattr.skel.h"
+
+#define CGROUP_FS_ROOT "/sys/fs/cgroup/"
+#define CGROUP_FS_PARENT CGROUP_FS_ROOT "foo/"
+#define CGROUP_FS_CHILD CGROUP_FS_PARENT "bar/"
+
+static int move_pid_to_cgroup(const char *cgroup_folder, pid_t pid)
+{
+ char filename[128];
+ char pid_str[64];
+ int procs_fd;
+ int ret;
+
+ snprintf(filename, sizeof(filename), "%scgroup.procs", cgroup_folder);
+ snprintf(pid_str, sizeof(pid_str), "%d", pid);
+
+ procs_fd = open(filename, O_WRONLY | O_APPEND);
+ if (!ASSERT_OK_FD(procs_fd, "open"))
+ return -1;
+
+ ret = write(procs_fd, pid_str, strlen(pid_str));
+ close(procs_fd);
+ if (!ASSERT_GT(ret, 0, "write cgroup.procs"))
+ return -1;
+ return 0;
+}
+
+static void reset_cgroups_and_lo(void)
+{
+ rmdir(CGROUP_FS_CHILD);
+ rmdir(CGROUP_FS_PARENT);
+ system("ip addr del 1.1.1.1/32 dev lo");
+ system("ip link set dev lo down");
+}
+
+static const char xattr_value_a[] = "bpf_selftest_value_a";
+static const char xattr_value_b[] = "bpf_selftest_value_b";
+static const char xattr_name[] = "user.bpf_test";
+
+static int setup_cgroups_and_lo(void)
+{
+ int err;
+
+ err = mkdir(CGROUP_FS_PARENT, 0755);
+ if (!ASSERT_OK(err, "mkdir 1"))
+ goto error;
+ err = mkdir(CGROUP_FS_CHILD, 0755);
+ if (!ASSERT_OK(err, "mkdir 2"))
+ goto error;
+
+ err = setxattr(CGROUP_FS_PARENT, xattr_name, xattr_value_a,
+ strlen(xattr_value_a) + 1, 0);
+ if (!ASSERT_OK(err, "setxattr 1"))
+ goto error;
+
+ err = setxattr(CGROUP_FS_CHILD, xattr_name, xattr_value_b,
+ strlen(xattr_value_b) + 1, 0);
+ if (!ASSERT_OK(err, "setxattr 2"))
+ goto error;
+
+ err = system("ip link set dev lo up");
+ if (!ASSERT_OK(err, "lo up"))
+ goto error;
+
+ err = system("ip addr add 1.1.1.1 dev lo");
+ if (!ASSERT_OK(err, "lo addr v4"))
+ goto error;
+
+ err = write_sysctl("/proc/sys/net/ipv4/ping_group_range", "0 0");
+ if (!ASSERT_OK(err, "write_sysctl"))
+ goto error;
+
+ return 0;
+error:
+ reset_cgroups_and_lo();
+ return err;
+}
+
+static void test_read_cgroup_xattr(void)
+{
+ struct sockaddr_in sa4 = {
+ .sin_family = AF_INET,
+ .sin_addr.s_addr = htonl(INADDR_LOOPBACK),
+ };
+ struct read_cgroupfs_xattr *skel = NULL;
+ pid_t pid = gettid();
+ int sock_fd = -1;
+ int connect_fd = -1;
+
+ if (!ASSERT_OK(setup_cgroups_and_lo(), "setup_cgroups_and_lo"))
+ return;
+ if (!ASSERT_OK(move_pid_to_cgroup(CGROUP_FS_CHILD, pid),
+ "move_pid_to_cgroup"))
+ goto out;
+
+ skel = read_cgroupfs_xattr__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "read_cgroupfs_xattr__open_and_load"))
+ goto out;
+
+ skel->bss->target_pid = pid;
+
+ if (!ASSERT_OK(read_cgroupfs_xattr__attach(skel), "read_cgroupfs_xattr__attach"))
+ goto out;
+
+ sock_fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP);
+ if (!ASSERT_OK_FD(sock_fd, "sock create"))
+ goto out;
+
+ connect_fd = connect(sock_fd, &sa4, sizeof(sa4));
+ if (!ASSERT_OK_FD(connect_fd, "connect 1"))
+ goto out;
+ close(connect_fd);
+
+ ASSERT_TRUE(skel->bss->found_value_a, "found_value_a");
+ ASSERT_TRUE(skel->bss->found_value_b, "found_value_b");
+
+out:
+ close(connect_fd);
+ close(sock_fd);
+ read_cgroupfs_xattr__destroy(skel);
+ move_pid_to_cgroup(CGROUP_FS_ROOT, pid);
+ reset_cgroups_and_lo();
+}
+
+void test_cgroup_xattr(void)
+{
+ RUN_TESTS(cgroup_read_xattr);
+
+ if (test__start_subtest("read_cgroupfs_xattr"))
+ test_read_cgroup_xattr();
+}
diff --git a/tools/testing/selftests/bpf/progs/cgroup_read_xattr.c b/tools/testing/selftests/bpf/progs/cgroup_read_xattr.c
new file mode 100644
index 000000000000..092db1d0435e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cgroup_read_xattr.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+#include "bpf_experimental.h"
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+char value[16];
+
+static __always_inline void read_xattr(struct cgroup *cgroup)
+{
+ struct bpf_dynptr value_ptr;
+
+ bpf_dynptr_from_mem(value, sizeof(value), 0, &value_ptr);
+ bpf_cgroup_read_xattr(cgroup, "user.bpf_test",
+ &value_ptr);
+}
+
+SEC("lsm.s/socket_connect")
+__success
+int BPF_PROG(trusted_cgroup_ptr_sleepable)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup *cgrp;
+
+ cgrp = bpf_cgroup_from_id(cgrp_id);
+ if (!cgrp)
+ return 0;
+
+ read_xattr(cgrp);
+ bpf_cgroup_release(cgrp);
+ return 0;
+}
+
+SEC("lsm/socket_connect")
+__success
+int BPF_PROG(trusted_cgroup_ptr_non_sleepable)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup *cgrp;
+
+ cgrp = bpf_cgroup_from_id(cgrp_id);
+ if (!cgrp)
+ return 0;
+
+ read_xattr(cgrp);
+ bpf_cgroup_release(cgrp);
+ return 0;
+}
+
+SEC("lsm/socket_connect")
+__success
+int BPF_PROG(use_css_iter_non_sleepable)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup_subsys_state *css;
+ struct cgroup *cgrp;
+
+ cgrp = bpf_cgroup_from_id(cgrp_id);
+ if (!cgrp)
+ return 0;
+
+ bpf_for_each(css, css, &cgrp->self, BPF_CGROUP_ITER_ANCESTORS_UP)
+ read_xattr(css->cgroup);
+
+ bpf_cgroup_release(cgrp);
+ return 0;
+}
+
+SEC("lsm.s/socket_connect")
+__failure __msg("expected an RCU CS")
+int BPF_PROG(use_css_iter_sleepable_missing_rcu_lock)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup_subsys_state *css;
+ struct cgroup *cgrp;
+
+ cgrp = bpf_cgroup_from_id(cgrp_id);
+ if (!cgrp)
+ return 0;
+
+ bpf_for_each(css, css, &cgrp->self, BPF_CGROUP_ITER_ANCESTORS_UP)
+ read_xattr(css->cgroup);
+
+ bpf_cgroup_release(cgrp);
+ return 0;
+}
+
+SEC("lsm.s/socket_connect")
+__success
+int BPF_PROG(use_css_iter_sleepable_with_rcu_lock)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup_subsys_state *css;
+ struct cgroup *cgrp;
+
+ bpf_rcu_read_lock();
+ cgrp = bpf_cgroup_from_id(cgrp_id);
+ if (!cgrp)
+ goto out;
+
+ bpf_for_each(css, css, &cgrp->self, BPF_CGROUP_ITER_ANCESTORS_UP)
+ read_xattr(css->cgroup);
+
+ bpf_cgroup_release(cgrp);
+out:
+ bpf_rcu_read_unlock();
+ return 0;
+}
+
+SEC("lsm/socket_connect")
+__success
+int BPF_PROG(use_bpf_cgroup_ancestor)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup *cgrp, *ancestor;
+
+ cgrp = bpf_cgroup_from_id(cgrp_id);
+ if (!cgrp)
+ return 0;
+
+ ancestor = bpf_cgroup_ancestor(cgrp, 1);
+ if (!ancestor)
+ goto out;
+
+ read_xattr(cgrp);
+ bpf_cgroup_release(ancestor);
+out:
+ bpf_cgroup_release(cgrp);
+ return 0;
+}
+
+SEC("cgroup/sendmsg4")
+__success
+int BPF_PROG(cgroup_skb)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup *cgrp, *ancestor;
+
+ cgrp = bpf_cgroup_from_id(cgrp_id);
+ if (!cgrp)
+ return 0;
+
+ ancestor = bpf_cgroup_ancestor(cgrp, 1);
+ if (!ancestor)
+ goto out;
+
+ read_xattr(cgrp);
+ bpf_cgroup_release(ancestor);
+out:
+ bpf_cgroup_release(cgrp);
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/read_cgroupfs_xattr.c b/tools/testing/selftests/bpf/progs/read_cgroupfs_xattr.c
new file mode 100644
index 000000000000..855f85fc5522
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/read_cgroupfs_xattr.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+#include "bpf_experimental.h"
+
+char _license[] SEC("license") = "GPL";
+
+pid_t target_pid = 0;
+
+char xattr_value[64];
+static const char expected_value_a[] = "bpf_selftest_value_a";
+static const char expected_value_b[] = "bpf_selftest_value_b";
+bool found_value_a;
+bool found_value_b;
+
+SEC("lsm.s/socket_connect")
+int BPF_PROG(test_socket_connect)
+{
+ u64 cgrp_id = bpf_get_current_cgroup_id();
+ struct cgroup_subsys_state *css, *tmp;
+ struct bpf_dynptr value_ptr;
+ struct cgroup *cgrp;
+
+ if ((bpf_get_current_pid_tgid() >> 32) != target_pid)
+ return 0;
+
+ bpf_rcu_read_lock();
+ cgrp = bpf_cgroup_from_id(cgrp_id);
+ if (!cgrp) {
+ bpf_rcu_read_unlock();
+ return 0;
+ }
+
+ css = &cgrp->self;
+ bpf_dynptr_from_mem(xattr_value, sizeof(xattr_value), 0, &value_ptr);
+ bpf_for_each(css, tmp, css, BPF_CGROUP_ITER_ANCESTORS_UP) {
+ int ret;
+
+ ret = bpf_cgroup_read_xattr(tmp->cgroup, "user.bpf_test",
+ &value_ptr);
+ if (ret < 0)
+ continue;
+
+ if (ret == sizeof(expected_value_a) &&
+ !bpf_strncmp(xattr_value, sizeof(expected_value_a), expected_value_a))
+ found_value_a = true;
+ if (ret == sizeof(expected_value_b) &&
+ !bpf_strncmp(xattr_value, sizeof(expected_value_b), expected_value_b))
+ found_value_b = true;
+ }
+
+ bpf_rcu_read_unlock();
+ bpf_cgroup_release(cgrp);
+
+ return 0;
+}
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-06-23 6:38 [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Song Liu
` (3 preceding siblings ...)
2025-06-23 6:38 ` [PATCH v3 bpf-next 4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr Song Liu
@ 2025-06-23 11:03 ` Christian Brauner
2025-06-27 2:14 ` Alexei Starovoitov
2025-06-27 2:20 ` patchwork-bot+netdevbpf
5 siblings, 1 reply; 23+ messages in thread
From: Christian Brauner @ 2025-06-23 11:03 UTC (permalink / raw)
To: Song Liu
Cc: Christian Brauner, kernel-team, andrii, eddyz87, ast, daniel,
martin.lau, viro, jack, kpsingh, mattbobrowski, amir73il, gregkh,
tj, daan.j.demeyer, bpf, linux-fsdevel, linux-kernel,
linux-security-module
On Sun, 22 Jun 2025 23:38:50 -0700, Song Liu wrote:
> Introduce a new kfunc bpf_cgroup_read_xattr, which can read xattr from
> cgroupfs nodes. The primary users are LSMs, cgroup programs, and sched_ext.
>
Applied to the vfs-6.17.bpf branch of the vfs/vfs.git tree.
Patches in the vfs-6.17.bpf branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.17.bpf
[1/4] kernfs: remove iattr_mutex
https://git.kernel.org/vfs/vfs/c/d1f4e9026007
[2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node
https://git.kernel.org/vfs/vfs/c/535b070f4a80
[3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe
https://git.kernel.org/vfs/vfs/c/1504d8c7c702
[4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr
https://git.kernel.org/vfs/vfs/c/f4fba2d6d282
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-06-23 11:03 ` [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Christian Brauner
@ 2025-06-27 2:14 ` Alexei Starovoitov
2025-06-27 4:04 ` Song Liu
2025-07-01 8:31 ` Christian Brauner
0 siblings, 2 replies; 23+ messages in thread
From: Alexei Starovoitov @ 2025-06-27 2:14 UTC (permalink / raw)
To: Christian Brauner
Cc: Song Liu, Kernel Team, Andrii Nakryiko, Eduard,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Mon, Jun 23, 2025 at 4:03 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Sun, 22 Jun 2025 23:38:50 -0700, Song Liu wrote:
> > Introduce a new kfunc bpf_cgroup_read_xattr, which can read xattr from
> > cgroupfs nodes. The primary users are LSMs, cgroup programs, and sched_ext.
> >
>
> Applied to the vfs-6.17.bpf branch of the vfs/vfs.git tree.
> Patches in the vfs-6.17.bpf branch should appear in linux-next soon.
Thanks.
Now merged into bpf-next/master as well.
> Please report any outstanding bugs that were missed during review in a
> new review to the original patch series allowing us to drop it.
bugs :(
> It's encouraged to provide Acked-bys and Reviewed-bys even though the
> patch has now been applied. If possible patch trailers will be updated.
Pls don't. Keep it as-is, otherwise there will be merge conflicts
during the merge window.
> Note that commit hashes shown below are subject to change due to rebase,
> trailer updates or similar. If in doubt, please check the listed branch.
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> branch: vfs-6.17.bpf
>
> [1/4] kernfs: remove iattr_mutex
> https://git.kernel.org/vfs/vfs/c/d1f4e9026007
> [2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node
> https://git.kernel.org/vfs/vfs/c/535b070f4a80
> [3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe
> https://git.kernel.org/vfs/vfs/c/1504d8c7c702
> [4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr
> https://git.kernel.org/vfs/vfs/c/f4fba2d6d282
Something wrong with this selftest.
Cleanup is not done correctly.
./test_progs -t lsm_cgroup
Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
./test_progs -t lsm_cgroup
Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
./test_progs -t cgroup_xattr
Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
./test_progs -t lsm_cgroup
test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
(network_helpers.c:121: errno: Cannot assign requested address) Failed
to bind socket
test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
actual -1 < expected 0
(network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
connect_to_fd: actual -1 < expected 0
test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
actual -1 < expected 0
test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
actual 0 != expected 234
...
Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
Song,
Please follow up with the fix for selftest.
It will be in bpf-next only.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-06-23 6:38 [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Song Liu
` (4 preceding siblings ...)
2025-06-23 11:03 ` [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Christian Brauner
@ 2025-06-27 2:20 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 23+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-06-27 2:20 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-fsdevel, linux-kernel, linux-security-module,
kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, amir73il, gregkh, tj,
daan.j.demeyer
Hello:
This series was applied to bpf/bpf-next.git (master)
by Christian Brauner <brauner@kernel.org>:
On Sun, 22 Jun 2025 23:38:50 -0700 you wrote:
> Introduce a new kfunc bpf_cgroup_read_xattr, which can read xattr from
> cgroupfs nodes. The primary users are LSMs, cgroup programs, and sched_ext.
>
> ---
>
> Changes v2 => v3:
> 1. Make bpf_cgroup_read_xattr available to all program types.
> 2. Fix gcc build warning on the selftests.
> 3. Add "ifdef CONFIG_CGROUPS" around bpf_cgroup_read_xattr.
>
> [...]
Here is the summary with links:
- [v3,bpf-next,1/4] kernfs: remove iattr_mutex
(no matching commit)
- [v3,bpf-next,2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node
https://git.kernel.org/bpf/bpf-next/c/535b070f4a80
- [v3,bpf-next,3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe
https://git.kernel.org/bpf/bpf-next/c/1504d8c7c702
- [v3,bpf-next,4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr
https://git.kernel.org/bpf/bpf-next/c/f4fba2d6d282
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-06-27 2:14 ` Alexei Starovoitov
@ 2025-06-27 4:04 ` Song Liu
2025-06-27 15:59 ` Alexei Starovoitov
2025-07-01 8:31 ` Christian Brauner
1 sibling, 1 reply; 23+ messages in thread
From: Song Liu @ 2025-06-27 4:04 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Christian Brauner, Kernel Team, Andrii Nakryiko, Eduard,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Thu, Jun 26, 2025 at 7:14 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
[...]
> ./test_progs -t lsm_cgroup
> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> ./test_progs -t lsm_cgroup
> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> ./test_progs -t cgroup_xattr
> Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
> ./test_progs -t lsm_cgroup
> test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
> (network_helpers.c:121: errno: Cannot assign requested address) Failed
> to bind socket
> test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
> actual -1 < expected 0
> (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
> test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
> connect_to_fd: actual -1 < expected 0
> test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
> test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
> actual -1 < expected 0
> test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
> actual 0 != expected 234
> ...
> Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
>
>
> Song,
> Please follow up with the fix for selftest.
> It will be in bpf-next only.
The issue is because cgroup_xattr calls "ip link set dev lo up"
in setup, and calls "ip link set dev lo down" in cleanup. Most
other tests only call "ip link set dev lo up". IOW, it appears to
me that cgroup_xattr is doing the cleanup properly. To fix this,
we can either remove "dev lo down" from cgroup_xattr, or add
"dev lo up" to lsm_cgroups. Do you have any preference one
way or another?
Thanks,
Song
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-06-27 4:04 ` Song Liu
@ 2025-06-27 15:59 ` Alexei Starovoitov
2025-06-27 16:20 ` Song Liu
0 siblings, 1 reply; 23+ messages in thread
From: Alexei Starovoitov @ 2025-06-27 15:59 UTC (permalink / raw)
To: Song Liu
Cc: Christian Brauner, Kernel Team, Andrii Nakryiko, Eduard,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Thu, Jun 26, 2025 at 9:04 PM Song Liu <song@kernel.org> wrote:
>
> On Thu, Jun 26, 2025 at 7:14 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> [...]
> > ./test_progs -t lsm_cgroup
> > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > ./test_progs -t lsm_cgroup
> > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > ./test_progs -t cgroup_xattr
> > Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
> > ./test_progs -t lsm_cgroup
> > test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
> > (network_helpers.c:121: errno: Cannot assign requested address) Failed
> > to bind socket
> > test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
> > actual -1 < expected 0
> > (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
> > test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
> > connect_to_fd: actual -1 < expected 0
> > test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
> > test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
> > actual -1 < expected 0
> > test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
> > actual 0 != expected 234
> > ...
> > Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
> >
> >
> > Song,
> > Please follow up with the fix for selftest.
> > It will be in bpf-next only.
>
> The issue is because cgroup_xattr calls "ip link set dev lo up"
> in setup, and calls "ip link set dev lo down" in cleanup. Most
> other tests only call "ip link set dev lo up". IOW, it appears to
> me that cgroup_xattr is doing the cleanup properly. To fix this,
> we can either remove "dev lo down" from cgroup_xattr, or add
> "dev lo up" to lsm_cgroups. Do you have any preference one
> way or another?
It messes with "lo" without switching netns? Ouch.
Not sure what tests you copied that code from,
but all "ip" commands, ping_group_range, and sockets
don't need to be in the test. Instead of triggering
progs through lsm/socket_connect hook can't you use
a simple hook like lsm/bpf or lsm/file_open that doesn't require
networking setup ?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-06-27 15:59 ` Alexei Starovoitov
@ 2025-06-27 16:20 ` Song Liu
2025-07-01 8:32 ` Christian Brauner
0 siblings, 1 reply; 23+ messages in thread
From: Song Liu @ 2025-06-27 16:20 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Song Liu, Christian Brauner, Kernel Team, Andrii Nakryiko, Eduard,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
> On Jun 27, 2025, at 8:59 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Jun 26, 2025 at 9:04 PM Song Liu <song@kernel.org> wrote:
>>
>> On Thu, Jun 26, 2025 at 7:14 PM Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>> [...]
>>> ./test_progs -t lsm_cgroup
>>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
>>> ./test_progs -t lsm_cgroup
>>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
>>> ./test_progs -t cgroup_xattr
>>> Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
>>> ./test_progs -t lsm_cgroup
>>> test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
>>> (network_helpers.c:121: errno: Cannot assign requested address) Failed
>>> to bind socket
>>> test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
>>> actual -1 < expected 0
>>> (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
>>> test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
>>> connect_to_fd: actual -1 < expected 0
>>> test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
>>> test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
>>> actual -1 < expected 0
>>> test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
>>> actual 0 != expected 234
>>> ...
>>> Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
>>>
>>>
>>> Song,
>>> Please follow up with the fix for selftest.
>>> It will be in bpf-next only.
>>
>> The issue is because cgroup_xattr calls "ip link set dev lo up"
>> in setup, and calls "ip link set dev lo down" in cleanup. Most
>> other tests only call "ip link set dev lo up". IOW, it appears to
>> me that cgroup_xattr is doing the cleanup properly. To fix this,
>> we can either remove "dev lo down" from cgroup_xattr, or add
>> "dev lo up" to lsm_cgroups. Do you have any preference one
>> way or another?
>
> It messes with "lo" without switching netns? Ouch.
Ah, I see the problem now.
> Not sure what tests you copied that code from,
> but all "ip" commands, ping_group_range, and sockets
> don't need to be in the test. Instead of triggering
> progs through lsm/socket_connect hook can't you use
> a simple hook like lsm/bpf or lsm/file_open that doesn't require
> networking setup ?
Yeah, let me fix the test with a different hook.
Thanks,
Song
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-06-27 2:14 ` Alexei Starovoitov
2025-06-27 4:04 ` Song Liu
@ 2025-07-01 8:31 ` Christian Brauner
2025-07-01 14:51 ` Alexei Starovoitov
1 sibling, 1 reply; 23+ messages in thread
From: Christian Brauner @ 2025-07-01 8:31 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Song Liu, Kernel Team, Andrii Nakryiko, Eduard,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Thu, Jun 26, 2025 at 07:14:20PM -0700, Alexei Starovoitov wrote:
> On Mon, Jun 23, 2025 at 4:03 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Sun, 22 Jun 2025 23:38:50 -0700, Song Liu wrote:
> > > Introduce a new kfunc bpf_cgroup_read_xattr, which can read xattr from
> > > cgroupfs nodes. The primary users are LSMs, cgroup programs, and sched_ext.
> > >
> >
> > Applied to the vfs-6.17.bpf branch of the vfs/vfs.git tree.
> > Patches in the vfs-6.17.bpf branch should appear in linux-next soon.
>
> Thanks.
> Now merged into bpf-next/master as well.
>
> > Please report any outstanding bugs that were missed during review in a
> > new review to the original patch series allowing us to drop it.
>
> bugs :(
>
> > It's encouraged to provide Acked-bys and Reviewed-bys even though the
> > patch has now been applied. If possible patch trailers will be updated.
>
> Pls don't. Keep it as-is, otherwise there will be merge conflicts
> during the merge window.
This is just the common blurb. As soon as another part of the tree
relies on something we stabilize the branch and only do fixes on top and
never rebase. We usually recommend just pulling the branch which I think
you did.
>
> > Note that commit hashes shown below are subject to change due to rebase,
> > trailer updates or similar. If in doubt, please check the listed branch.
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> > branch: vfs-6.17.bpf
> >
> > [1/4] kernfs: remove iattr_mutex
> > https://git.kernel.org/vfs/vfs/c/d1f4e9026007
> > [2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node
> > https://git.kernel.org/vfs/vfs/c/535b070f4a80
> > [3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe
> > https://git.kernel.org/vfs/vfs/c/1504d8c7c702
> > [4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr
> > https://git.kernel.org/vfs/vfs/c/f4fba2d6d282
>
> Something wrong with this selftest.
> Cleanup is not done correctly.
>
> ./test_progs -t lsm_cgroup
> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> ./test_progs -t lsm_cgroup
> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> ./test_progs -t cgroup_xattr
> Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
> ./test_progs -t lsm_cgroup
> test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
> (network_helpers.c:121: errno: Cannot assign requested address) Failed
> to bind socket
> test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
> actual -1 < expected 0
> (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
> test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
> connect_to_fd: actual -1 < expected 0
> test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
> test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
> actual -1 < expected 0
> test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
> actual 0 != expected 234
> ...
> Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
>
>
> Song,
> Please follow up with the fix for selftest.
> It will be in bpf-next only.
We should put that commit on the shared vfs-6.17.bpf branch.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-06-27 16:20 ` Song Liu
@ 2025-07-01 8:32 ` Christian Brauner
2025-07-01 16:23 ` Song Liu
0 siblings, 1 reply; 23+ messages in thread
From: Christian Brauner @ 2025-07-01 8:32 UTC (permalink / raw)
To: Song Liu
Cc: Alexei Starovoitov, Song Liu, Kernel Team, Andrii Nakryiko,
Eduard, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Fri, Jun 27, 2025 at 04:20:58PM +0000, Song Liu wrote:
>
>
> > On Jun 27, 2025, at 8:59 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >
> > On Thu, Jun 26, 2025 at 9:04 PM Song Liu <song@kernel.org> wrote:
> >>
> >> On Thu, Jun 26, 2025 at 7:14 PM Alexei Starovoitov
> >> <alexei.starovoitov@gmail.com> wrote:
> >> [...]
> >>> ./test_progs -t lsm_cgroup
> >>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> >>> ./test_progs -t lsm_cgroup
> >>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> >>> ./test_progs -t cgroup_xattr
> >>> Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
> >>> ./test_progs -t lsm_cgroup
> >>> test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
> >>> (network_helpers.c:121: errno: Cannot assign requested address) Failed
> >>> to bind socket
> >>> test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
> >>> actual -1 < expected 0
> >>> (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
> >>> test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
> >>> connect_to_fd: actual -1 < expected 0
> >>> test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
> >>> test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
> >>> actual -1 < expected 0
> >>> test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
> >>> actual 0 != expected 234
> >>> ...
> >>> Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
> >>>
> >>>
> >>> Song,
> >>> Please follow up with the fix for selftest.
> >>> It will be in bpf-next only.
> >>
> >> The issue is because cgroup_xattr calls "ip link set dev lo up"
> >> in setup, and calls "ip link set dev lo down" in cleanup. Most
> >> other tests only call "ip link set dev lo up". IOW, it appears to
> >> me that cgroup_xattr is doing the cleanup properly. To fix this,
> >> we can either remove "dev lo down" from cgroup_xattr, or add
> >> "dev lo up" to lsm_cgroups. Do you have any preference one
> >> way or another?
> >
> > It messes with "lo" without switching netns? Ouch.
>
> Ah, I see the problem now.
>
> > Not sure what tests you copied that code from,
> > but all "ip" commands, ping_group_range, and sockets
> > don't need to be in the test. Instead of triggering
> > progs through lsm/socket_connect hook can't you use
> > a simple hook like lsm/bpf or lsm/file_open that doesn't require
> > networking setup ?
>
> Yeah, let me fix the test with a different hook.
Where's the patch?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-07-01 8:31 ` Christian Brauner
@ 2025-07-01 14:51 ` Alexei Starovoitov
2025-07-02 8:37 ` Christian Brauner
0 siblings, 1 reply; 23+ messages in thread
From: Alexei Starovoitov @ 2025-07-01 14:51 UTC (permalink / raw)
To: Christian Brauner
Cc: Song Liu, Kernel Team, Andrii Nakryiko, Eduard,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Tue, Jul 1, 2025 at 1:32 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Thu, Jun 26, 2025 at 07:14:20PM -0700, Alexei Starovoitov wrote:
> > On Mon, Jun 23, 2025 at 4:03 AM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > On Sun, 22 Jun 2025 23:38:50 -0700, Song Liu wrote:
> > > > Introduce a new kfunc bpf_cgroup_read_xattr, which can read xattr from
> > > > cgroupfs nodes. The primary users are LSMs, cgroup programs, and sched_ext.
> > > >
> > >
> > > Applied to the vfs-6.17.bpf branch of the vfs/vfs.git tree.
> > > Patches in the vfs-6.17.bpf branch should appear in linux-next soon.
> >
> > Thanks.
> > Now merged into bpf-next/master as well.
> >
> > > Please report any outstanding bugs that were missed during review in a
> > > new review to the original patch series allowing us to drop it.
> >
> > bugs :(
> >
> > > It's encouraged to provide Acked-bys and Reviewed-bys even though the
> > > patch has now been applied. If possible patch trailers will be updated.
> >
> > Pls don't. Keep it as-is, otherwise there will be merge conflicts
> > during the merge window.
>
> This is just the common blurb. As soon as another part of the tree
> relies on something we stabilize the branch and only do fixes on top and
> never rebase. We usually recommend just pulling the branch which I think
> you did.
>
> >
> > > Note that commit hashes shown below are subject to change due to rebase,
> > > trailer updates or similar. If in doubt, please check the listed branch.
> > >
> > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> > > branch: vfs-6.17.bpf
> > >
> > > [1/4] kernfs: remove iattr_mutex
> > > https://git.kernel.org/vfs/vfs/c/d1f4e9026007
> > > [2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node
> > > https://git.kernel.org/vfs/vfs/c/535b070f4a80
> > > [3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe
> > > https://git.kernel.org/vfs/vfs/c/1504d8c7c702
> > > [4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr
> > > https://git.kernel.org/vfs/vfs/c/f4fba2d6d282
> >
> > Something wrong with this selftest.
> > Cleanup is not done correctly.
> >
> > ./test_progs -t lsm_cgroup
> > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > ./test_progs -t lsm_cgroup
> > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > ./test_progs -t cgroup_xattr
> > Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
> > ./test_progs -t lsm_cgroup
> > test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
> > (network_helpers.c:121: errno: Cannot assign requested address) Failed
> > to bind socket
> > test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
> > actual -1 < expected 0
> > (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
> > test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
> > connect_to_fd: actual -1 < expected 0
> > test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
> > test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
> > actual -1 < expected 0
> > test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
> > actual 0 != expected 234
> > ...
> > Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
> >
> >
> > Song,
> > Please follow up with the fix for selftest.
> > It will be in bpf-next only.
>
> We should put that commit on the shared vfs-6.17.bpf branch.
The branch had a conflict with bpf-next which was resolved
in the merge commit. Then _two_ fixes were applied on top.
And one fix is right where conflict was.
So it's not possible to apply both fixes to vfs-6.17.bpf.
imo this shared branch experience wasn't good.
We should have applied the series to bpf-next only.
It was more bpf material than vfs. I wouldn't do this again.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-07-01 8:32 ` Christian Brauner
@ 2025-07-01 16:23 ` Song Liu
2025-07-02 12:19 ` Christian Brauner
0 siblings, 1 reply; 23+ messages in thread
From: Song Liu @ 2025-07-01 16:23 UTC (permalink / raw)
To: Christian Brauner
Cc: Song Liu, Alexei Starovoitov, Kernel Team, Andrii Nakryiko,
Eduard, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Tue, Jul 1, 2025 at 1:32 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Jun 27, 2025 at 04:20:58PM +0000, Song Liu wrote:
> >
> >
> > > On Jun 27, 2025, at 8:59 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Thu, Jun 26, 2025 at 9:04 PM Song Liu <song@kernel.org> wrote:
> > >>
> > >> On Thu, Jun 26, 2025 at 7:14 PM Alexei Starovoitov
> > >> <alexei.starovoitov@gmail.com> wrote:
> > >> [...]
> > >>> ./test_progs -t lsm_cgroup
> > >>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > >>> ./test_progs -t lsm_cgroup
> > >>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > >>> ./test_progs -t cgroup_xattr
> > >>> Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
> > >>> ./test_progs -t lsm_cgroup
> > >>> test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
> > >>> (network_helpers.c:121: errno: Cannot assign requested address) Failed
> > >>> to bind socket
> > >>> test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
> > >>> actual -1 < expected 0
> > >>> (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
> > >>> test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
> > >>> connect_to_fd: actual -1 < expected 0
> > >>> test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
> > >>> test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
> > >>> actual -1 < expected 0
> > >>> test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
> > >>> actual 0 != expected 234
> > >>> ...
> > >>> Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
> > >>>
> > >>>
> > >>> Song,
> > >>> Please follow up with the fix for selftest.
> > >>> It will be in bpf-next only.
> > >>
> > >> The issue is because cgroup_xattr calls "ip link set dev lo up"
> > >> in setup, and calls "ip link set dev lo down" in cleanup. Most
> > >> other tests only call "ip link set dev lo up". IOW, it appears to
> > >> me that cgroup_xattr is doing the cleanup properly. To fix this,
> > >> we can either remove "dev lo down" from cgroup_xattr, or add
> > >> "dev lo up" to lsm_cgroups. Do you have any preference one
> > >> way or another?
> > >
> > > It messes with "lo" without switching netns? Ouch.
> >
> > Ah, I see the problem now.
> >
> > > Not sure what tests you copied that code from,
> > > but all "ip" commands, ping_group_range, and sockets
> > > don't need to be in the test. Instead of triggering
> > > progs through lsm/socket_connect hook can't you use
> > > a simple hook like lsm/bpf or lsm/file_open that doesn't require
> > > networking setup ?
> >
> > Yeah, let me fix the test with a different hook.
>
> Where's the patch?
Here is a fix to kernel/bpf/helprs.c by Eduard:
https://lore.kernel.org/bpf/20250627175309.2710973-1-eddyz87@gmail.com/
This fix addresses build errors with certain config.
Here is my fix to the selftests:
https://lore.kernel.org/bpf/20250627191221.765921-1-song@kernel.org/
I didn't CC linux-fsdevel because all the changes are in the
selftests, and the error is independent of the new code.
Thanks,
Song
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-07-01 14:51 ` Alexei Starovoitov
@ 2025-07-02 8:37 ` Christian Brauner
0 siblings, 0 replies; 23+ messages in thread
From: Christian Brauner @ 2025-07-02 8:37 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Song Liu, Kernel Team, Andrii Nakryiko, Eduard,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Tue, Jul 01, 2025 at 07:51:55AM -0700, Alexei Starovoitov wrote:
> On Tue, Jul 1, 2025 at 1:32 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Thu, Jun 26, 2025 at 07:14:20PM -0700, Alexei Starovoitov wrote:
> > > On Mon, Jun 23, 2025 at 4:03 AM Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > On Sun, 22 Jun 2025 23:38:50 -0700, Song Liu wrote:
> > > > > Introduce a new kfunc bpf_cgroup_read_xattr, which can read xattr from
> > > > > cgroupfs nodes. The primary users are LSMs, cgroup programs, and sched_ext.
> > > > >
> > > >
> > > > Applied to the vfs-6.17.bpf branch of the vfs/vfs.git tree.
> > > > Patches in the vfs-6.17.bpf branch should appear in linux-next soon.
> > >
> > > Thanks.
> > > Now merged into bpf-next/master as well.
> > >
> > > > Please report any outstanding bugs that were missed during review in a
> > > > new review to the original patch series allowing us to drop it.
> > >
> > > bugs :(
> > >
> > > > It's encouraged to provide Acked-bys and Reviewed-bys even though the
> > > > patch has now been applied. If possible patch trailers will be updated.
> > >
> > > Pls don't. Keep it as-is, otherwise there will be merge conflicts
> > > during the merge window.
> >
> > This is just the common blurb. As soon as another part of the tree
> > relies on something we stabilize the branch and only do fixes on top and
> > never rebase. We usually recommend just pulling the branch which I think
> > you did.
> >
> > >
> > > > Note that commit hashes shown below are subject to change due to rebase,
> > > > trailer updates or similar. If in doubt, please check the listed branch.
> > > >
> > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> > > > branch: vfs-6.17.bpf
> > > >
> > > > [1/4] kernfs: remove iattr_mutex
> > > > https://git.kernel.org/vfs/vfs/c/d1f4e9026007
> > > > [2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node
> > > > https://git.kernel.org/vfs/vfs/c/535b070f4a80
> > > > [3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe
> > > > https://git.kernel.org/vfs/vfs/c/1504d8c7c702
> > > > [4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr
> > > > https://git.kernel.org/vfs/vfs/c/f4fba2d6d282
> > >
> > > Something wrong with this selftest.
> > > Cleanup is not done correctly.
> > >
> > > ./test_progs -t lsm_cgroup
> > > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > > ./test_progs -t lsm_cgroup
> > > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > > ./test_progs -t cgroup_xattr
> > > Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
> > > ./test_progs -t lsm_cgroup
> > > test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
> > > (network_helpers.c:121: errno: Cannot assign requested address) Failed
> > > to bind socket
> > > test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
> > > actual -1 < expected 0
> > > (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
> > > test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
> > > connect_to_fd: actual -1 < expected 0
> > > test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
> > > test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
> > > actual -1 < expected 0
> > > test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
> > > actual 0 != expected 234
> > > ...
> > > Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
> > >
> > >
> > > Song,
> > > Please follow up with the fix for selftest.
> > > It will be in bpf-next only.
> >
> > We should put that commit on the shared vfs-6.17.bpf branch.
>
> The branch had a conflict with bpf-next which was resolved
> in the merge commit. Then _two_ fixes were applied on top.
> And one fix is right where conflict was.
> So it's not possible to apply both fixes to vfs-6.17.bpf.
> imo this shared branch experience wasn't good.
> We should have applied the series to bpf-next only.
> It was more bpf material than vfs. I wouldn't do this again.
Absolutely not. Anything that touches VFS will go through VFS. Shared
branches work just fine. We manage to do this with everyone else in the
kernel so bpf is able to do this as well. If you'd just asked this would
not have been an issue. Merge conflicts are a fact of kernel
development, we all deal with it you can too.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex
2025-06-23 6:38 ` [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex Song Liu
@ 2025-07-02 10:47 ` André Draszik
2025-07-02 12:17 ` Christian Brauner
0 siblings, 1 reply; 23+ messages in thread
From: André Draszik @ 2025-07-02 10:47 UTC (permalink / raw)
To: Song Liu, bpf, linux-fsdevel, linux-kernel, linux-security-module
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, amir73il, gregkh, tj,
daan.j.demeyer, Will McVicker, Peter Griffin, Tudor Ambarus,
kernel-team
Hi,
On Sun, 2025-06-22 at 23:38 -0700, Song Liu wrote:
> From: Christian Brauner <brauner@kernel.org>
>
> All allocations of struct kernfs_iattrs are serialized through a global
> mutex. Simply do a racy allocation and let the first one win. I bet most
> callers are under inode->i_rwsem anyway and it wouldn't be needed but
> let's not require that.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Song Liu <song@kernel.org>
On next-20250701, ls -lA gives errors on /sys:
$ ls -lA /sys/
ls: /sys/: No data available
ls: /sys/kernel: No data available
ls: /sys/power: No data available
ls: /sys/class: No data available
ls: /sys/devices: No data available
ls: /sys/dev: No data available
ls: /sys/hypervisor: No data available
ls: /sys/fs: No data available
ls: /sys/bus: No data available
ls: /sys/firmware: No data available
ls: /sys/block: No data available
ls: /sys/module: No data available
total 0
drwxr-xr-x 2 root root 0 Jan 1 1970 block
drwxr-xr-x 52 root root 0 Jan 1 1970 bus
drwxr-xr-x 88 root root 0 Jan 1 1970 class
drwxr-xr-x 4 root root 0 Jan 1 1970 dev
drwxr-xr-x 11 root root 0 Jan 1 1970 devices
drwxr-xr-x 3 root root 0 Jan 1 1970 firmware
drwxr-xr-x 10 root root 0 Jan 1 1970 fs
drwxr-xr-x 2 root root 0 Jul 2 09:43 hypervisor
drwxr-xr-x 14 root root 0 Jan 1 1970 kernel
drwxr-xr-x 251 root root 0 Jan 1 1970 module
drwxr-xr-x 3 root root 0 Jul 2 09:43 power
and my bisect is pointing to this commit. Simply reverting it also fixes
the errors.
Do you have any suggestions?
Cheers,
Andre'
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex
2025-07-02 10:47 ` André Draszik
@ 2025-07-02 12:17 ` Christian Brauner
2025-07-03 6:28 ` André Draszik
2025-08-16 5:52 ` Jan Kiszka
0 siblings, 2 replies; 23+ messages in thread
From: Christian Brauner @ 2025-07-02 12:17 UTC (permalink / raw)
To: André Draszik
Cc: Song Liu, bpf, linux-fsdevel, linux-kernel, linux-security-module,
kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro, jack,
kpsingh, mattbobrowski, amir73il, gregkh, tj, daan.j.demeyer,
Will McVicker, Peter Griffin, Tudor Ambarus, kernel-team
[-- Attachment #1: Type: text/plain, Size: 4686 bytes --]
On Wed, Jul 02, 2025 at 11:47:58AM +0100, André Draszik wrote:
> Hi,
>
> On Sun, 2025-06-22 at 23:38 -0700, Song Liu wrote:
> > From: Christian Brauner <brauner@kernel.org>
> >
> > All allocations of struct kernfs_iattrs are serialized through a global
> > mutex. Simply do a racy allocation and let the first one win. I bet most
> > callers are under inode->i_rwsem anyway and it wouldn't be needed but
> > let's not require that.
> >
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Acked-by: Tejun Heo <tj@kernel.org>
> > Signed-off-by: Song Liu <song@kernel.org>
>
> On next-20250701, ls -lA gives errors on /sys:
>
> $ ls -lA /sys/
> ls: /sys/: No data available
> ls: /sys/kernel: No data available
> ls: /sys/power: No data available
> ls: /sys/class: No data available
> ls: /sys/devices: No data available
> ls: /sys/dev: No data available
> ls: /sys/hypervisor: No data available
> ls: /sys/fs: No data available
> ls: /sys/bus: No data available
> ls: /sys/firmware: No data available
> ls: /sys/block: No data available
> ls: /sys/module: No data available
> total 0
> drwxr-xr-x 2 root root 0 Jan 1 1970 block
> drwxr-xr-x 52 root root 0 Jan 1 1970 bus
> drwxr-xr-x 88 root root 0 Jan 1 1970 class
> drwxr-xr-x 4 root root 0 Jan 1 1970 dev
> drwxr-xr-x 11 root root 0 Jan 1 1970 devices
> drwxr-xr-x 3 root root 0 Jan 1 1970 firmware
> drwxr-xr-x 10 root root 0 Jan 1 1970 fs
> drwxr-xr-x 2 root root 0 Jul 2 09:43 hypervisor
> drwxr-xr-x 14 root root 0 Jan 1 1970 kernel
> drwxr-xr-x 251 root root 0 Jan 1 1970 module
> drwxr-xr-x 3 root root 0 Jul 2 09:43 power
>
>
> and my bisect is pointing to this commit. Simply reverting it also fixes
> the errors.
>
>
> Do you have any suggestions?
Yes, apparently the xattr selftest don't cover sysfs/kernfs. The issue
is that the commit changed listxattr() to skip allocation of the xattr
header and instead just returned ENODATA. We should just allocate like
before tested just now:
user1@localhost:~$ sudo ls -al /sys/kernel/
total 0
drwxr-xr-x 17 root root 0 Jul 2 13:41 .
dr-xr-xr-x 12 root root 0 Jul 2 13:41 ..
-r--r--r-- 1 root root 4096 Jul 2 13:41 address_bits
drwxr-xr-x 3 root root 0 Jul 2 13:41 boot_params
drwxr-xr-x 2 root root 0 Jul 2 13:41 btf
drwxr-xr-x 2 root root 0 Jul 2 13:41 cgroup
drwxr-xr-x 2 root root 0 Jul 2 13:41 config
-r--r--r-- 1 root root 4096 Jul 2 13:41 cpu_byteorder
-r--r--r-- 1 root root 4096 Jul 2 13:41 crash_elfcorehdr_size
drwx------ 34 root root 0 Jul 2 13:41 debug
-r--r--r-- 1 root root 4096 Jul 2 13:41 fscaps
-r--r--r-- 1 root root 4096 Jul 2 13:41 hardlockup_count
drwxr-xr-x 2 root root 0 Jul 2 13:41 iommu_groups
drwxr-xr-x 344 root root 0 Jul 2 13:41 irq
-r--r--r-- 1 root root 4096 Jul 2 13:41 kexec_crash_loaded
-rw-r--r-- 1 root root 4096 Jul 2 13:41 kexec_crash_size
-r--r--r-- 1 root root 4096 Jul 2 13:41 kexec_loaded
drwxr-xr-x 9 root root 0 Jul 2 13:41 mm
-r--r--r-- 1 root root 84 Jul 2 13:41 notes
-r--r--r-- 1 root root 4096 Jul 2 13:41 oops_count
-rw-r--r-- 1 root root 4096 Jul 2 13:41 profiling
-rw-r--r-- 1 root root 4096 Jul 2 13:41 rcu_expedited
-rw-r--r-- 1 root root 4096 Jul 2 13:41 rcu_normal
-r--r--r-- 1 root root 4096 Jul 2 13:41 rcu_stall_count
drwxr-xr-x 2 root root 0 Jul 2 13:41 reboot
drwxr-xr-x 2 root root 0 Jul 2 13:41 sched_ext
drwxr-xr-x 4 root root 0 Jul 2 13:41 security
drwxr-xr-x 190 root root 0 Jul 2 13:41 slab
-r--r--r-- 1 root root 4096 Jul 2 13:41 softlockup_count
drwxr-xr-x 2 root root 0 Jul 2 13:41 software_nodes
drwxr-xr-x 4 root root 0 Jul 2 13:41 sunrpc
drwxr-xr-x 6 root root 0 Jul 2 13:41 tracing
-r--r--r-- 1 root root 4096 Jul 2 13:41 uevent_seqnum
-r--r--r-- 1 root root 4096 Jul 2 13:41 vmcoreinfo
-r--r--r-- 1 root root 4096 Jul 2 13:41 warn_count
I'm folding:
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 3c293a5a21b1..457f91c412d4 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -142,9 +142,9 @@ ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
struct kernfs_node *kn = kernfs_dentry_node(dentry);
struct kernfs_iattrs *attrs;
- attrs = kernfs_iattrs_noalloc(kn);
+ attrs = kernfs_iattrs(kn);
if (!attrs)
- return -ENODATA;
+ return -ENOMEM;
return simple_xattr_list(d_inode(dentry), &attrs->xattrs, buf, size);
}
which brings it back to the old behavior.
I'm also adding a selftest for this behavior. Patch appended.
[-- Attachment #2: 0001-selftests-kernfs-test-xattr-retrieval.patch --]
[-- Type: text/x-diff, Size: 2746 bytes --]
From c20804314ae1ca5678e6b135b0ab1bc54fb3e410 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Wed, 2 Jul 2025 13:53:21 +0200
Subject: [PATCH] selftests/kernfs: test xattr retrieval
Make sure that listxattr() returns zero and that getxattr() returns
ENODATA when no extended attributs are set. Use /sys/kernel/warn_count
as that always exists and is a read-only file.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
.../testing/selftests/filesystems/.gitignore | 1 +
tools/testing/selftests/filesystems/Makefile | 2 +-
.../selftests/filesystems/kernfs_test.c | 38 +++++++++++++++++++
3 files changed, 40 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/filesystems/kernfs_test.c
diff --git a/tools/testing/selftests/filesystems/.gitignore b/tools/testing/selftests/filesystems/.gitignore
index 7afa58e2bb20..fcbdb1297e24 100644
--- a/tools/testing/selftests/filesystems/.gitignore
+++ b/tools/testing/selftests/filesystems/.gitignore
@@ -3,3 +3,4 @@ dnotify_test
devpts_pts
file_stressor
anon_inode_test
+kernfs_test
diff --git a/tools/testing/selftests/filesystems/Makefile b/tools/testing/selftests/filesystems/Makefile
index b02326193fee..73d4650af1a5 100644
--- a/tools/testing/selftests/filesystems/Makefile
+++ b/tools/testing/selftests/filesystems/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS += $(KHDR_INCLUDES)
-TEST_GEN_PROGS := devpts_pts file_stressor anon_inode_test
+TEST_GEN_PROGS := devpts_pts file_stressor anon_inode_test kernfs_test
TEST_GEN_PROGS_EXTENDED := dnotify_test
include ../lib.mk
diff --git a/tools/testing/selftests/filesystems/kernfs_test.c b/tools/testing/selftests/filesystems/kernfs_test.c
new file mode 100644
index 000000000000..16538b3b318e
--- /dev/null
+++ b/tools/testing/selftests/filesystems/kernfs_test.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#define __SANE_USERSPACE_TYPES__
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <sys/stat.h>
+#include <sys/xattr.h>
+
+#include "../kselftest_harness.h"
+#include "wrappers.h"
+
+TEST(kernfs_listxattr)
+{
+ int fd;
+
+ /* Read-only file that can never have any extended attributes set. */
+ fd = open("/sys/kernel/warn_count", O_RDONLY | O_CLOEXEC);
+ ASSERT_GE(fd, 0);
+ ASSERT_EQ(flistxattr(fd, NULL, 0), 0);
+ EXPECT_EQ(close(fd), 0);
+}
+
+TEST(kernfs_getxattr)
+{
+ int fd;
+ char buf[1];
+
+ /* Read-only file that can never have any extended attributes set. */
+ fd = open("/sys/kernel/warn_count", O_RDONLY | O_CLOEXEC);
+ ASSERT_GE(fd, 0);
+ ASSERT_LT(fgetxattr(fd, "user.foo", buf, sizeof(buf)), 0);
+ ASSERT_EQ(errno, ENODATA);
+ EXPECT_EQ(close(fd), 0);
+}
+
+TEST_HARNESS_MAIN
+
--
2.47.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr
2025-07-01 16:23 ` Song Liu
@ 2025-07-02 12:19 ` Christian Brauner
0 siblings, 0 replies; 23+ messages in thread
From: Christian Brauner @ 2025-07-02 12:19 UTC (permalink / raw)
To: Song Liu
Cc: Song Liu, Alexei Starovoitov, Kernel Team, Andrii Nakryiko,
Eduard, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Alexander Viro, Jan Kara, KP Singh, Matt Bobrowski,
Amir Goldstein, Greg Kroah-Hartman, Tejun Heo, Daan De Meyer, bpf,
Linux-Fsdevel, LKML, LSM List
On Tue, Jul 01, 2025 at 09:23:30AM -0700, Song Liu wrote:
> On Tue, Jul 1, 2025 at 1:32 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Fri, Jun 27, 2025 at 04:20:58PM +0000, Song Liu wrote:
> > >
> > >
> > > > On Jun 27, 2025, at 8:59 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Thu, Jun 26, 2025 at 9:04 PM Song Liu <song@kernel.org> wrote:
> > > >>
> > > >> On Thu, Jun 26, 2025 at 7:14 PM Alexei Starovoitov
> > > >> <alexei.starovoitov@gmail.com> wrote:
> > > >> [...]
> > > >>> ./test_progs -t lsm_cgroup
> > > >>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > > >>> ./test_progs -t lsm_cgroup
> > > >>> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
> > > >>> ./test_progs -t cgroup_xattr
> > > >>> Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
> > > >>> ./test_progs -t lsm_cgroup
> > > >>> test_lsm_cgroup_functional:PASS:bind(ETH_P_ALL) 0 nsec
> > > >>> (network_helpers.c:121: errno: Cannot assign requested address) Failed
> > > >>> to bind socket
> > > >>> test_lsm_cgroup_functional:FAIL:start_server unexpected start_server:
> > > >>> actual -1 < expected 0
> > > >>> (network_helpers.c:360: errno: Bad file descriptor) getsockopt(SOL_PROTOCOL)
> > > >>> test_lsm_cgroup_functional:FAIL:connect_to_fd unexpected
> > > >>> connect_to_fd: actual -1 < expected 0
> > > >>> test_lsm_cgroup_functional:FAIL:accept unexpected accept: actual -1 < expected 0
> > > >>> test_lsm_cgroup_functional:FAIL:getsockopt unexpected getsockopt:
> > > >>> actual -1 < expected 0
> > > >>> test_lsm_cgroup_functional:FAIL:sk_priority unexpected sk_priority:
> > > >>> actual 0 != expected 234
> > > >>> ...
> > > >>> Summary: 0/1 PASSED, 0 SKIPPED, 1 FAILED
> > > >>>
> > > >>>
> > > >>> Song,
> > > >>> Please follow up with the fix for selftest.
> > > >>> It will be in bpf-next only.
> > > >>
> > > >> The issue is because cgroup_xattr calls "ip link set dev lo up"
> > > >> in setup, and calls "ip link set dev lo down" in cleanup. Most
> > > >> other tests only call "ip link set dev lo up". IOW, it appears to
> > > >> me that cgroup_xattr is doing the cleanup properly. To fix this,
> > > >> we can either remove "dev lo down" from cgroup_xattr, or add
> > > >> "dev lo up" to lsm_cgroups. Do you have any preference one
> > > >> way or another?
> > > >
> > > > It messes with "lo" without switching netns? Ouch.
> > >
> > > Ah, I see the problem now.
> > >
> > > > Not sure what tests you copied that code from,
> > > > but all "ip" commands, ping_group_range, and sockets
> > > > don't need to be in the test. Instead of triggering
> > > > progs through lsm/socket_connect hook can't you use
> > > > a simple hook like lsm/bpf or lsm/file_open that doesn't require
> > > > networking setup ?
> > >
> > > Yeah, let me fix the test with a different hook.
> >
> > Where's the patch?
>
> Here is a fix to kernel/bpf/helprs.c by Eduard:
> https://lore.kernel.org/bpf/20250627175309.2710973-1-eddyz87@gmail.com/
>
> This fix addresses build errors with certain config.
>
> Here is my fix to the selftests:
> https://lore.kernel.org/bpf/20250627191221.765921-1-song@kernel.org/
>
> I didn't CC linux-fsdevel because all the changes are in the
> selftests, and the error is independent of the new code.
Ah great, thank you for the info. That helped and we don't have to care then.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex
2025-07-02 12:17 ` Christian Brauner
@ 2025-07-03 6:28 ` André Draszik
2025-08-16 5:52 ` Jan Kiszka
1 sibling, 0 replies; 23+ messages in thread
From: André Draszik @ 2025-07-03 6:28 UTC (permalink / raw)
To: Christian Brauner
Cc: Song Liu, bpf, linux-fsdevel, linux-kernel, linux-security-module,
kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro, jack,
kpsingh, mattbobrowski, amir73il, gregkh, tj, daan.j.demeyer,
Will McVicker, Peter Griffin, Tudor Ambarus, kernel-team
On Wed, 2025-07-02 at 14:17 +0200, Christian Brauner wrote:
> I'm folding:
>
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index 3c293a5a21b1..457f91c412d4 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -142,9 +142,9 @@ ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
> struct kernfs_node *kn = kernfs_dentry_node(dentry);
> struct kernfs_iattrs *attrs;
>
> - attrs = kernfs_iattrs_noalloc(kn);
> + attrs = kernfs_iattrs(kn);
> if (!attrs)
> - return -ENODATA;
> + return -ENOMEM;
>
> return simple_xattr_list(d_inode(dentry), &attrs->xattrs, buf, size);
> }
>
> which brings it back to the old behavior.
Yes, that makes sense and works for me too.
Thanks Christian!
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex
2025-07-02 12:17 ` Christian Brauner
2025-07-03 6:28 ` André Draszik
@ 2025-08-16 5:52 ` Jan Kiszka
2025-08-19 10:05 ` Christian Brauner
1 sibling, 1 reply; 23+ messages in thread
From: Jan Kiszka @ 2025-08-16 5:52 UTC (permalink / raw)
To: Christian Brauner, André Draszik
Cc: Song Liu, bpf, linux-fsdevel, linux-kernel, linux-security-module,
kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro, jack,
kpsingh, mattbobrowski, amir73il, gregkh, tj, daan.j.demeyer,
Will McVicker, Peter Griffin, Tudor Ambarus, kernel-team
On 02.07.25 14:17, Christian Brauner wrote:
> On Wed, Jul 02, 2025 at 11:47:58AM +0100, André Draszik wrote:
>> Hi,
>>
>> On Sun, 2025-06-22 at 23:38 -0700, Song Liu wrote:
>>> From: Christian Brauner <brauner@kernel.org>
>>>
>>> All allocations of struct kernfs_iattrs are serialized through a global
>>> mutex. Simply do a racy allocation and let the first one win. I bet most
>>> callers are under inode->i_rwsem anyway and it wouldn't be needed but
>>> let's not require that.
>>>
>>> Signed-off-by: Christian Brauner <brauner@kernel.org>
>>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Acked-by: Tejun Heo <tj@kernel.org>
>>> Signed-off-by: Song Liu <song@kernel.org>
>>
>> On next-20250701, ls -lA gives errors on /sys:
>>
>> $ ls -lA /sys/
>> ls: /sys/: No data available
>> ls: /sys/kernel: No data available
>> ls: /sys/power: No data available
>> ls: /sys/class: No data available
>> ls: /sys/devices: No data available
>> ls: /sys/dev: No data available
>> ls: /sys/hypervisor: No data available
>> ls: /sys/fs: No data available
>> ls: /sys/bus: No data available
>> ls: /sys/firmware: No data available
>> ls: /sys/block: No data available
>> ls: /sys/module: No data available
>> total 0
>> drwxr-xr-x 2 root root 0 Jan 1 1970 block
>> drwxr-xr-x 52 root root 0 Jan 1 1970 bus
>> drwxr-xr-x 88 root root 0 Jan 1 1970 class
>> drwxr-xr-x 4 root root 0 Jan 1 1970 dev
>> drwxr-xr-x 11 root root 0 Jan 1 1970 devices
>> drwxr-xr-x 3 root root 0 Jan 1 1970 firmware
>> drwxr-xr-x 10 root root 0 Jan 1 1970 fs
>> drwxr-xr-x 2 root root 0 Jul 2 09:43 hypervisor
>> drwxr-xr-x 14 root root 0 Jan 1 1970 kernel
>> drwxr-xr-x 251 root root 0 Jan 1 1970 module
>> drwxr-xr-x 3 root root 0 Jul 2 09:43 power
>>
>>
>> and my bisect is pointing to this commit. Simply reverting it also fixes
>> the errors.
>>
>>
>> Do you have any suggestions?
>
> Yes, apparently the xattr selftest don't cover sysfs/kernfs. The issue
> is that the commit changed listxattr() to skip allocation of the xattr
> header and instead just returned ENODATA. We should just allocate like
> before tested just now:
>
> user1@localhost:~$ sudo ls -al /sys/kernel/
> total 0
> drwxr-xr-x 17 root root 0 Jul 2 13:41 .
> dr-xr-xr-x 12 root root 0 Jul 2 13:41 ..
> -r--r--r-- 1 root root 4096 Jul 2 13:41 address_bits
> drwxr-xr-x 3 root root 0 Jul 2 13:41 boot_params
> drwxr-xr-x 2 root root 0 Jul 2 13:41 btf
> drwxr-xr-x 2 root root 0 Jul 2 13:41 cgroup
> drwxr-xr-x 2 root root 0 Jul 2 13:41 config
> -r--r--r-- 1 root root 4096 Jul 2 13:41 cpu_byteorder
> -r--r--r-- 1 root root 4096 Jul 2 13:41 crash_elfcorehdr_size
> drwx------ 34 root root 0 Jul 2 13:41 debug
> -r--r--r-- 1 root root 4096 Jul 2 13:41 fscaps
> -r--r--r-- 1 root root 4096 Jul 2 13:41 hardlockup_count
> drwxr-xr-x 2 root root 0 Jul 2 13:41 iommu_groups
> drwxr-xr-x 344 root root 0 Jul 2 13:41 irq
> -r--r--r-- 1 root root 4096 Jul 2 13:41 kexec_crash_loaded
> -rw-r--r-- 1 root root 4096 Jul 2 13:41 kexec_crash_size
> -r--r--r-- 1 root root 4096 Jul 2 13:41 kexec_loaded
> drwxr-xr-x 9 root root 0 Jul 2 13:41 mm
> -r--r--r-- 1 root root 84 Jul 2 13:41 notes
> -r--r--r-- 1 root root 4096 Jul 2 13:41 oops_count
> -rw-r--r-- 1 root root 4096 Jul 2 13:41 profiling
> -rw-r--r-- 1 root root 4096 Jul 2 13:41 rcu_expedited
> -rw-r--r-- 1 root root 4096 Jul 2 13:41 rcu_normal
> -r--r--r-- 1 root root 4096 Jul 2 13:41 rcu_stall_count
> drwxr-xr-x 2 root root 0 Jul 2 13:41 reboot
> drwxr-xr-x 2 root root 0 Jul 2 13:41 sched_ext
> drwxr-xr-x 4 root root 0 Jul 2 13:41 security
> drwxr-xr-x 190 root root 0 Jul 2 13:41 slab
> -r--r--r-- 1 root root 4096 Jul 2 13:41 softlockup_count
> drwxr-xr-x 2 root root 0 Jul 2 13:41 software_nodes
> drwxr-xr-x 4 root root 0 Jul 2 13:41 sunrpc
> drwxr-xr-x 6 root root 0 Jul 2 13:41 tracing
> -r--r--r-- 1 root root 4096 Jul 2 13:41 uevent_seqnum
> -r--r--r-- 1 root root 4096 Jul 2 13:41 vmcoreinfo
> -r--r--r-- 1 root root 4096 Jul 2 13:41 warn_count
>
> I'm folding:
>
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index 3c293a5a21b1..457f91c412d4 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -142,9 +142,9 @@ ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
> struct kernfs_node *kn = kernfs_dentry_node(dentry);
> struct kernfs_iattrs *attrs;
>
> - attrs = kernfs_iattrs_noalloc(kn);
> + attrs = kernfs_iattrs(kn);
> if (!attrs)
> - return -ENODATA;
> + return -ENOMEM;
>
> return simple_xattr_list(d_inode(dentry), &attrs->xattrs, buf, size);
> }
>
> which brings it back to the old behavior.
>
...but it looks like v3 was merged as-is in the end, without this fixup.
Is there some separate patch in the pipeline, or was this forgotten?
> I'm also adding a selftest for this behavior. Patch appended.
Jan
--
Siemens AG, Foundational Technologies
Linux Expert Center
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex
2025-08-16 5:52 ` Jan Kiszka
@ 2025-08-19 10:05 ` Christian Brauner
2025-08-19 10:08 ` [PATCH] kernfs: don't fail listing extended attributes Christian Brauner
0 siblings, 1 reply; 23+ messages in thread
From: Christian Brauner @ 2025-08-19 10:05 UTC (permalink / raw)
To: Jan Kiszka
Cc: André Draszik, Song Liu, bpf, linux-fsdevel, linux-kernel,
linux-security-module, kernel-team, andrii, eddyz87, ast, daniel,
martin.lau, viro, jack, kpsingh, mattbobrowski, amir73il, gregkh,
tj, daan.j.demeyer, Will McVicker, Peter Griffin, Tudor Ambarus,
kernel-team
> ...but it looks like v3 was merged as-is in the end, without this fixup.
> Is there some separate patch in the pipeline, or was this forgotten?
This is a result of the trees diverging which we discussed earlier.
I sent a fix.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] kernfs: don't fail listing extended attributes
2025-08-19 10:05 ` Christian Brauner
@ 2025-08-19 10:08 ` Christian Brauner
0 siblings, 0 replies; 23+ messages in thread
From: Christian Brauner @ 2025-08-19 10:08 UTC (permalink / raw)
To: Jan Kiszka
Cc: Christian Brauner, André Draszik, Song Liu, bpf,
linux-fsdevel, andrii, ast, daniel
Userspace doesn't expect a failure to list extended attributes:
$ ls -lA /sys/
ls: /sys/: No data available
ls: /sys/kernel: No data available
ls: /sys/power: No data available
ls: /sys/class: No data available
ls: /sys/devices: No data available
ls: /sys/dev: No data available
ls: /sys/hypervisor: No data available
ls: /sys/fs: No data available
ls: /sys/bus: No data available
ls: /sys/firmware: No data available
ls: /sys/block: No data available
ls: /sys/module: No data available
total 0
drwxr-xr-x 2 root root 0 Jan 1 1970 block
drwxr-xr-x 52 root root 0 Jan 1 1970 bus
drwxr-xr-x 88 root root 0 Jan 1 1970 class
drwxr-xr-x 4 root root 0 Jan 1 1970 dev
drwxr-xr-x 11 root root 0 Jan 1 1970 devices
drwxr-xr-x 3 root root 0 Jan 1 1970 firmware
drwxr-xr-x 10 root root 0 Jan 1 1970 fs
drwxr-xr-x 2 root root 0 Jul 2 09:43 hypervisor
drwxr-xr-x 14 root root 0 Jan 1 1970 kernel
drwxr-xr-x 251 root root 0 Jan 1 1970 module
drwxr-xr-x 3 root root 0 Jul 2 09:43 power
Fix it by simply reporting success when no extended attributes are
available instead of reporting ENODATA.
Fixes: d1f4e9026007 ("kernfs: remove iattr_mutex") # mainline only
Reported-by: André Draszik <andre.draszik@linaro.org>
Link: https://lore.kernel.org/78b13bcdae82ade95e88f315682966051f461dde.camel@linaro.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
fs/kernfs/inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 3c293a5a21b1..457f91c412d4 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -142,9 +142,9 @@ ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
struct kernfs_node *kn = kernfs_dentry_node(dentry);
struct kernfs_iattrs *attrs;
- attrs = kernfs_iattrs_noalloc(kn);
+ attrs = kernfs_iattrs(kn);
if (!attrs)
- return -ENODATA;
+ return -ENOMEM;
return simple_xattr_list(d_inode(dentry), &attrs->xattrs, buf, size);
}
--
2.47.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-08-19 10:09 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-23 6:38 [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 1/4] kernfs: remove iattr_mutex Song Liu
2025-07-02 10:47 ` André Draszik
2025-07-02 12:17 ` Christian Brauner
2025-07-03 6:28 ` André Draszik
2025-08-16 5:52 ` Jan Kiszka
2025-08-19 10:05 ` Christian Brauner
2025-08-19 10:08 ` [PATCH] kernfs: don't fail listing extended attributes Christian Brauner
2025-06-23 6:38 ` [PATCH v3 bpf-next 2/4] bpf: Introduce bpf_cgroup_read_xattr to read xattr of cgroup's node Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 3/4] bpf: Mark cgroup_subsys_state->cgroup RCU safe Song Liu
2025-06-23 6:38 ` [PATCH v3 bpf-next 4/4] selftests/bpf: Add tests for bpf_cgroup_read_xattr Song Liu
2025-06-23 11:03 ` [PATCH v3 bpf-next 0/4] Introduce bpf_cgroup_read_xattr Christian Brauner
2025-06-27 2:14 ` Alexei Starovoitov
2025-06-27 4:04 ` Song Liu
2025-06-27 15:59 ` Alexei Starovoitov
2025-06-27 16:20 ` Song Liu
2025-07-01 8:32 ` Christian Brauner
2025-07-01 16:23 ` Song Liu
2025-07-02 12:19 ` Christian Brauner
2025-07-01 8:31 ` Christian Brauner
2025-07-01 14:51 ` Alexei Starovoitov
2025-07-02 8:37 ` Christian Brauner
2025-06-27 2:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).