* [PATCH v2 bpf-next 0/2] security.bpf xattr name prefix
@ 2024-10-16 7:09 Song Liu
2024-10-16 7:09 ` [PATCH v2 bpf-next 1/2] fs/xattr: bpf: Introduce " Song Liu
2024-10-16 7:09 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Extend test fs_kfuncs to cover security.bpf xattr names Song Liu
0 siblings, 2 replies; 3+ messages in thread
From: Song Liu @ 2024-10-16 7:09 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-kernel
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, Song Liu
Follow up discussion in LPC 2024 [1], that we need security.bpf xattr
prefix. This set adds "security.bpf" xattr name prefix, and allows
bpf kfuncs bpf_get_[file|dentry]_xattr() to read these xattrs.
[1] https://lpc.events/event/18/contributions/1940/
Changes v1 => v2
1. Update comment of bpf_get_[file|dentry]_xattr. (Jiri Olsa)
2. Fix comment for return value of bpf_get_[file|dentry]_xattr.
v1: https://lore.kernel.org/bpf/20241002214637.3625277-1-song@kernel.org/
Song Liu (2):
fs/xattr: bpf: Introduce security.bpf xattr name prefix
selftests/bpf: Extend test fs_kfuncs to cover security.bpf xattr names
fs/bpf_fs_kfuncs.c | 22 +++++++++-
include/uapi/linux/xattr.h | 4 ++
.../selftests/bpf/prog_tests/fs_kfuncs.c | 40 ++++++++++++++-----
.../selftests/bpf/progs/test_get_xattr.c | 30 ++++++++++++--
4 files changed, 80 insertions(+), 16 deletions(-)
--
2.43.5
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 bpf-next 1/2] fs/xattr: bpf: Introduce security.bpf xattr name prefix
2024-10-16 7:09 [PATCH v2 bpf-next 0/2] security.bpf xattr name prefix Song Liu
@ 2024-10-16 7:09 ` Song Liu
2024-10-16 7:09 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Extend test fs_kfuncs to cover security.bpf xattr names Song Liu
1 sibling, 0 replies; 3+ messages in thread
From: Song Liu @ 2024-10-16 7:09 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-kernel
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, Song Liu
Introduct new xattr name prefix security.bpf, and enable reading these
xattrs from bpf kfuncs bpf_get_[file|dentry]_xattr(). Note that
"security.bpf" could be the name of the xattr or the prefix of the
name. For example, both "security.bpf" and "security.bpf.xxx" are
valid xattr name; while "security.bpfxxx" is not valid.
As we are on it, correct the comments for return value of
bpf_get_[file|dentry]_xattr(), i.e. return length the xattr value on
successl.
Signed-off-by: Song Liu <song@kernel.org>
Acked-by: Christian Brauner <brauner@kernel.org>
---
fs/bpf_fs_kfuncs.c | 29 ++++++++++++++++++++++++-----
include/uapi/linux/xattr.h | 4 ++++
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 3fe9f59ef867..be78a13f1d82 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -93,6 +93,23 @@ __bpf_kfunc int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz)
return len;
}
+static bool bpf_xattr_name_allowed(const char *name__str)
+{
+ /* Allow xattr names with user. prefix */
+ if (!strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
+ return true;
+
+ /* Allow security.bpf. prefix or just security.bpf */
+ if (!strncmp(name__str, XATTR_NAME_BPF_LSM, XATTR_NAME_BPF_LSM_LEN) &&
+ (name__str[XATTR_NAME_BPF_LSM_LEN] == '\0' ||
+ name__str[XATTR_NAME_BPF_LSM_LEN] == '.')) {
+ return true;
+ }
+
+ /* Disallow anything else */
+ return false;
+}
+
/**
* bpf_get_dentry_xattr - get xattr of a dentry
* @dentry: dentry to get xattr from
@@ -101,9 +118,10 @@ __bpf_kfunc int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz)
*
* Get xattr *name__str* of *dentry* and store the output in *value_ptr*.
*
- * For security reasons, only *name__str* with prefix "user." is allowed.
+ * For security reasons, only *name__str* with prefix "user." or
+ * "security.bpf" is allowed.
*
- * Return: 0 on success, a negative value on error.
+ * Return: length of the xattr value on success, a negative value on error.
*/
__bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__str,
struct bpf_dynptr *value_p)
@@ -117,7 +135,7 @@ __bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__st
if (WARN_ON(!inode))
return -EINVAL;
- if (strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
+ if (!bpf_xattr_name_allowed(name__str))
return -EPERM;
value_len = __bpf_dynptr_size(value_ptr);
@@ -139,9 +157,10 @@ __bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__st
*
* Get xattr *name__str* of *file* and store the output in *value_ptr*.
*
- * For security reasons, only *name__str* with prefix "user." is allowed.
+ * For security reasons, only *name__str* with prefix "user." or
+ * "security.bpf" is allowed.
*
- * Return: 0 on success, a negative value on error.
+ * Return: length of the xattr value on success, a negative value on error.
*/
__bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,
struct bpf_dynptr *value_p)
diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
index 9463db2dfa9d..166ef2f1f1b3 100644
--- a/include/uapi/linux/xattr.h
+++ b/include/uapi/linux/xattr.h
@@ -76,6 +76,10 @@
#define XATTR_CAPS_SUFFIX "capability"
#define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
+#define XATTR_BPF_LSM_SUFFIX "bpf"
+#define XATTR_NAME_BPF_LSM (XATTR_SECURITY_PREFIX XATTR_BPF_LSM_SUFFIX)
+#define XATTR_NAME_BPF_LSM_LEN (sizeof(XATTR_NAME_BPF_LSM) - 1)
+
#define XATTR_POSIX_ACL_ACCESS "posix_acl_access"
#define XATTR_NAME_POSIX_ACL_ACCESS XATTR_SYSTEM_PREFIX XATTR_POSIX_ACL_ACCESS
#define XATTR_POSIX_ACL_DEFAULT "posix_acl_default"
--
2.43.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2 bpf-next 2/2] selftests/bpf: Extend test fs_kfuncs to cover security.bpf xattr names
2024-10-16 7:09 [PATCH v2 bpf-next 0/2] security.bpf xattr name prefix Song Liu
2024-10-16 7:09 ` [PATCH v2 bpf-next 1/2] fs/xattr: bpf: Introduce " Song Liu
@ 2024-10-16 7:09 ` Song Liu
1 sibling, 0 replies; 3+ messages in thread
From: Song Liu @ 2024-10-16 7:09 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-kernel
Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
brauner, jack, kpsingh, mattbobrowski, Song Liu
Extend test_progs fs_kfuncs to cover different xattr names. Specifically:
xattr name "user.kfuncs", "security.bpf", and "security.bpf.xxx" can be
read from BPF program with kfuncs bpf_get_[file|dentry]_xattr(); while
"security.bpfxxx" and "security.selinux" cannot be read.
Signed-off-by: Song Liu <song@kernel.org>
---
.../selftests/bpf/prog_tests/fs_kfuncs.c | 40 ++++++++++++++-----
.../selftests/bpf/progs/test_get_xattr.c | 30 ++++++++++++--
2 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
index 5a0b51157451..986dd5eabaa6 100644
--- a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
+++ b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
@@ -12,7 +12,7 @@
static const char testfile[] = "/tmp/test_progs_fs_kfuncs";
-static void test_xattr(void)
+static void test_get_xattr(const char *name, const char *value, bool allow_access)
{
struct test_get_xattr *skel = NULL;
int fd = -1, err;
@@ -25,7 +25,7 @@ static void test_xattr(void)
close(fd);
fd = -1;
- err = setxattr(testfile, "user.kfuncs", "hello", sizeof("hello"), 0);
+ err = setxattr(testfile, name, value, strlen(value) + 1, 0);
if (err && errno == EOPNOTSUPP) {
printf("%s:SKIP:local fs doesn't support xattr (%d)\n"
"To run this test, make sure /tmp filesystem supports xattr.\n",
@@ -48,16 +48,23 @@ static void test_xattr(void)
goto out;
fd = open(testfile, O_RDONLY, 0644);
+
if (!ASSERT_GE(fd, 0, "open_file"))
goto out;
- ASSERT_EQ(skel->bss->found_xattr_from_file, 1, "found_xattr_from_file");
-
/* Trigger security_inode_getxattr */
- err = getxattr(testfile, "user.kfuncs", v, sizeof(v));
- ASSERT_EQ(err, -1, "getxattr_return");
- ASSERT_EQ(errno, EINVAL, "getxattr_errno");
- ASSERT_EQ(skel->bss->found_xattr_from_dentry, 1, "found_xattr_from_dentry");
+ err = getxattr(testfile, name, v, sizeof(v));
+
+ if (allow_access) {
+ ASSERT_EQ(err, -1, "getxattr_return");
+ ASSERT_EQ(errno, EINVAL, "getxattr_errno");
+ ASSERT_EQ(skel->bss->found_xattr_from_file, 1, "found_xattr_from_file");
+ ASSERT_EQ(skel->bss->found_xattr_from_dentry, 1, "found_xattr_from_dentry");
+ } else {
+ ASSERT_EQ(err, strlen(value) + 1, "getxattr_return");
+ ASSERT_EQ(skel->bss->found_xattr_from_file, 0, "found_xattr_from_file");
+ ASSERT_EQ(skel->bss->found_xattr_from_dentry, 0, "found_xattr_from_dentry");
+ }
out:
close(fd);
@@ -141,8 +148,21 @@ static void test_fsverity(void)
void test_fs_kfuncs(void)
{
- if (test__start_subtest("xattr"))
- test_xattr();
+ /* Matches xattr_names in progs/test_get_xattr.c */
+ if (test__start_subtest("user_xattr"))
+ test_get_xattr("user.kfuncs", "hello", true);
+
+ if (test__start_subtest("security_bpf_xattr_1"))
+ test_get_xattr("security.bpf", "hello", true);
+
+ if (test__start_subtest("security_bpf_xattr_2"))
+ test_get_xattr("security.bpf.xxx", "hello", true);
+
+ if (test__start_subtest("security_bpfxxx_xattr_error"))
+ test_get_xattr("security.bpfxxx", "hello", false);
+
+ if (test__start_subtest("security_selinux_xattr_error"))
+ test_get_xattr("security.selinux", "hello", false);
if (test__start_subtest("fsverity"))
test_fsverity();
diff --git a/tools/testing/selftests/bpf/progs/test_get_xattr.c b/tools/testing/selftests/bpf/progs/test_get_xattr.c
index 66e737720f7c..0be8120683cd 100644
--- a/tools/testing/selftests/bpf/progs/test_get_xattr.c
+++ b/tools/testing/selftests/bpf/progs/test_get_xattr.c
@@ -17,12 +17,26 @@ static const char expected_value[] = "hello";
char value1[32];
char value2[32];
+#define NUM_OF_XATTR_NAME 5
+
+/* Matches caller of test_get_xattr() in prog_tests/fs_kfuncs.c */
+static const char *xattr_names[NUM_OF_XATTR_NAME] = {
+ /* The following work. */
+ "user.kfuncs",
+ "security.bpf",
+ "security.bpf.xxx",
+
+ /* The following do not work. */
+ "security.bpfxxx",
+ "security.selinux"
+};
+
SEC("lsm.s/file_open")
int BPF_PROG(test_file_open, struct file *f)
{
struct bpf_dynptr value_ptr;
__u32 pid;
- int ret;
+ int ret, i;
pid = bpf_get_current_pid_tgid() >> 32;
if (pid != monitored_pid)
@@ -30,7 +44,11 @@ int BPF_PROG(test_file_open, struct file *f)
bpf_dynptr_from_mem(value1, sizeof(value1), 0, &value_ptr);
- ret = bpf_get_file_xattr(f, "user.kfuncs", &value_ptr);
+ for (i = 0; i < NUM_OF_XATTR_NAME; i++) {
+ ret = bpf_get_file_xattr(f, xattr_names[i], &value_ptr);
+ if (ret == sizeof(expected_value))
+ break;
+ }
if (ret != sizeof(expected_value))
return 0;
if (bpf_strncmp(value1, ret, expected_value))
@@ -44,7 +62,7 @@ int BPF_PROG(test_inode_getxattr, struct dentry *dentry, char *name)
{
struct bpf_dynptr value_ptr;
__u32 pid;
- int ret;
+ int ret, i;
pid = bpf_get_current_pid_tgid() >> 32;
if (pid != monitored_pid)
@@ -52,7 +70,11 @@ int BPF_PROG(test_inode_getxattr, struct dentry *dentry, char *name)
bpf_dynptr_from_mem(value2, sizeof(value2), 0, &value_ptr);
- ret = bpf_get_dentry_xattr(dentry, "user.kfuncs", &value_ptr);
+ for (i = 0; i < NUM_OF_XATTR_NAME; i++) {
+ ret = bpf_get_dentry_xattr(dentry, xattr_names[i], &value_ptr);
+ if (ret == sizeof(expected_value))
+ break;
+ }
if (ret != sizeof(expected_value))
return 0;
if (bpf_strncmp(value2, ret, expected_value))
--
2.43.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-16 7:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-16 7:09 [PATCH v2 bpf-next 0/2] security.bpf xattr name prefix Song Liu
2024-10-16 7:09 ` [PATCH v2 bpf-next 1/2] fs/xattr: bpf: Introduce " Song Liu
2024-10-16 7:09 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Extend test fs_kfuncs to cover security.bpf xattr names Song Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.