* [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs
@ 2025-01-08 22:51 Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 1/7] fs/xattr: bpf: Introduce security.bpf. xattr name prefix Song Liu
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Song Liu @ 2025-01-08 22:51 UTC (permalink / raw)
To: bpf, linux-kernel, linux-security-module
Cc: kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor, Song Liu
Add support to set and remove xattr from BPF program. Also add
security.bpf. xattr name prefix.
kfuncs are added to set and remove xattrs with security.bpf. name
prefix. Update kfuncs bpf_get_[file|dentry]_xattr to read xattrs
with security.bpf. name prefix. Note that BPF programs can read
user. xattrs, but not write and remove them.
To pick the right version of kfunc to use, a remap logic is added to
btf_kfunc_id_set. This helps move some kfunc specific logic off the
verifier core code. Also use this remap logic to select
bpf_dynptr_from_skb or bpf_dynptr_from_skb_rdonly.
Cover letter of v1 and v2:
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 v7 => v8
1. Rebase and resolve conflicts.
v7: https://lore.kernel.org/bpf/20241219221439.2455664-1-song@kernel.org/
Changes v6 => v7
1. Move btf_kfunc_id_remap() to the right place. (Bug reported by CI)
v6: https://lore.kernel.org/bpf/20241219202536.1625216-1-song@kernel.org/
Changes v5 => v6
1. Hide _locked version of the kfuncs from vmlinux.h (Alexei)
2. Add remap logic to btf_kfunc_id_set and use that to pick the correct
version of kfuncs to use.
3. Also use the remap logic for bpf_dynptr_from_skb[|_rdonly].
v5: https://lore.kernel.org/bpf/20241218044711.1723221-1-song@kernel.org/
Changes v4 => v5
1. Let verifier pick proper kfunc (_locked or not _locked) based on the
calling context. (Alexei)
2. Remove the __failure test (6/6 of v4).
v4: https://lore.kernel.org/bpf/20241217063821.482857-1-song@kernel.org/
Changes v3 => v4
1. Do write permission check with inode locked. (Jan Kara)
2. Fix some source_inline warnings.
v3: https://lore.kernel.org/bpf/20241210220627.2800362-1-song@kernel.org/
Changes v2 => v3
1. Add kfuncs to set and remove xattr from BPF programs.
v2: https://lore.kernel.org/bpf/20241016070955.375923-1-song@kernel.org/
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 (7):
fs/xattr: bpf: Introduce security.bpf. xattr name prefix
selftests/bpf: Extend test fs_kfuncs to cover security.bpf. xattr
names
bpf: lsm: Add two more sleepable hooks
bpf: Extend btf_kfunc_id_set to handle kfunc polymorphism
bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
bpf: fs/xattr: Add BPF kfuncs to set and remove xattrs
selftests/bpf: Test kfuncs that set and remove xattr from BPF programs
fs/bpf_fs_kfuncs.c | 246 +++++++++++++++++-
include/linux/bpf_lsm.h | 2 +
include/linux/btf.h | 20 ++
include/linux/btf_ids.h | 3 +
include/uapi/linux/xattr.h | 4 +
kernel/bpf/bpf_lsm.c | 2 +
kernel/bpf/btf.c | 117 +++++++--
kernel/bpf/verifier.c | 31 +--
net/core/filter.c | 49 +++-
tools/testing/selftests/bpf/bpf_kfuncs.h | 5 +
.../selftests/bpf/prog_tests/fs_kfuncs.c | 162 +++++++++++-
.../selftests/bpf/progs/test_get_xattr.c | 28 +-
.../bpf/progs/test_set_remove_xattr.c | 133 ++++++++++
13 files changed, 739 insertions(+), 63 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/test_set_remove_xattr.c
--
2.43.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v8 bpf-next 1/7] fs/xattr: bpf: Introduce security.bpf. xattr name prefix
2025-01-08 22:51 [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs Song Liu
@ 2025-01-08 22:51 ` Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 2/7] selftests/bpf: Extend test fs_kfuncs to cover security.bpf. xattr names Song Liu
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2025-01-08 22:51 UTC (permalink / raw)
To: bpf, linux-kernel, linux-security-module
Cc: kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor, Song Liu,
Christian Brauner, Jan Kara
Introduct new xattr name prefix security.bpf., and enable reading these
xattrs from bpf kfuncs bpf_get_[file|dentry]_xattr().
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
success.
Signed-off-by: Song Liu <song@kernel.org>
Acked-by: Christian Brauner <brauner@kernel.org>
Reviewed-by: Jan Kara <jack@suse.cz>
---
fs/bpf_fs_kfuncs.c | 19 ++++++++++++++-----
include/uapi/linux/xattr.h | 4 ++++
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 3fe9f59ef867..8a65184c8c2c 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -93,6 +93,11 @@ __bpf_kfunc int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz)
return len;
}
+static bool match_security_bpf_prefix(const char *name__str)
+{
+ return !strncmp(name__str, XATTR_NAME_BPF_LSM, XATTR_NAME_BPF_LSM_LEN);
+}
+
/**
* bpf_get_dentry_xattr - get xattr of a dentry
* @dentry: dentry to get xattr from
@@ -101,9 +106,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 +123,9 @@ __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))
+ /* Allow reading xattr with user. and security.bpf. prefix */
+ if (strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN) &&
+ !match_security_bpf_prefix(name__str))
return -EPERM;
value_len = __bpf_dynptr_size(value_ptr);
@@ -139,9 +147,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 9854f9cff3c6..c7c85bb504ba 100644
--- a/include/uapi/linux/xattr.h
+++ b/include/uapi/linux/xattr.h
@@ -83,6 +83,10 @@ struct xattr_args {
#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] 14+ messages in thread
* [PATCH v8 bpf-next 2/7] selftests/bpf: Extend test fs_kfuncs to cover security.bpf. xattr names
2025-01-08 22:51 [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 1/7] fs/xattr: bpf: Introduce security.bpf. xattr name prefix Song Liu
@ 2025-01-08 22:51 ` Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 3/7] bpf: lsm: Add two more sleepable hooks Song Liu
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2025-01-08 22:51 UTC (permalink / raw)
To: bpf, linux-kernel, linux-security-module
Cc: kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor, Song Liu
Extend test_progs fs_kfuncs to cover different xattr names. Specifically:
xattr name "user.kfuncs" and "security.bpf.xxx" can be read from BPF
program with kfuncs bpf_get_[file|dentry]_xattr(); while "security.bpf"
and "security.selinux" cannot be read.
Signed-off-by: Song Liu <song@kernel.org>
---
.../selftests/bpf/prog_tests/fs_kfuncs.c | 37 ++++++++++++++-----
.../selftests/bpf/progs/test_get_xattr.c | 28 ++++++++++++--
2 files changed, 51 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..419f45b56472 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,18 @@ 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"))
+ test_get_xattr("security.bpf.xxx", "hello", true);
+
+ if (test__start_subtest("security_bpf_xattr_error"))
+ test_get_xattr("security.bpf", "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..358e3506e5b0 100644
--- a/tools/testing/selftests/bpf/progs/test_get_xattr.c
+++ b/tools/testing/selftests/bpf/progs/test_get_xattr.c
@@ -6,6 +6,7 @@
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
#include "bpf_kfuncs.h"
+#include "bpf_misc.h"
char _license[] SEC("license") = "GPL";
@@ -17,12 +18,23 @@ static const char expected_value[] = "hello";
char value1[32];
char value2[32];
+/* Matches caller of test_get_xattr() in prog_tests/fs_kfuncs.c */
+static const char * const xattr_names[] = {
+ /* The following work. */
+ "user.kfuncs",
+ "security.bpf.xxx",
+
+ /* The following do not work. */
+ "security.bpf",
+ "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 +42,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 < ARRAY_SIZE(xattr_names); 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 +60,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 +68,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 < ARRAY_SIZE(xattr_names); 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] 14+ messages in thread
* [PATCH v8 bpf-next 3/7] bpf: lsm: Add two more sleepable hooks
2025-01-08 22:51 [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 1/7] fs/xattr: bpf: Introduce security.bpf. xattr name prefix Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 2/7] selftests/bpf: Extend test fs_kfuncs to cover security.bpf. xattr names Song Liu
@ 2025-01-08 22:51 ` Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 4/7] bpf: Extend btf_kfunc_id_set to handle kfunc polymorphism Song Liu
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2025-01-08 22:51 UTC (permalink / raw)
To: bpf, linux-kernel, linux-security-module
Cc: kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor, Song Liu
Add bpf_lsm_inode_removexattr and bpf_lsm_inode_post_removexattr to list
sleepable_lsm_hooks. These two hooks are always called from sleepable
context.
Signed-off-by: Song Liu <song@kernel.org>
---
kernel/bpf/bpf_lsm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 967492b65185..0a59df1c550a 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -316,7 +316,9 @@ BTF_ID(func, bpf_lsm_inode_getxattr)
BTF_ID(func, bpf_lsm_inode_mknod)
BTF_ID(func, bpf_lsm_inode_need_killpriv)
BTF_ID(func, bpf_lsm_inode_post_setxattr)
+BTF_ID(func, bpf_lsm_inode_post_removexattr)
BTF_ID(func, bpf_lsm_inode_readlink)
+BTF_ID(func, bpf_lsm_inode_removexattr)
BTF_ID(func, bpf_lsm_inode_rename)
BTF_ID(func, bpf_lsm_inode_rmdir)
BTF_ID(func, bpf_lsm_inode_setattr)
--
2.43.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v8 bpf-next 4/7] bpf: Extend btf_kfunc_id_set to handle kfunc polymorphism
2025-01-08 22:51 [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs Song Liu
` (2 preceding siblings ...)
2025-01-08 22:51 ` [PATCH v8 bpf-next 3/7] bpf: lsm: Add two more sleepable hooks Song Liu
@ 2025-01-08 22:51 ` Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb Song Liu
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2025-01-08 22:51 UTC (permalink / raw)
To: bpf, linux-kernel, linux-security-module
Cc: kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor, Song Liu
Polymorphism exists in kernel functions, BPF helpers, as well as kfuncs.
When called from different contexts, it is necessary to pick the right
version of a kfunc. One of such example is bpf_dynptr_from_skb vs.
bpf_dynptr_from_skb_rdonly.
To avoid the burden on the users, the verifier can inspect the calling
context and select the right version of kfunc. However, with more kfuncs
being added to the kernel, it is not scalable to push all these logic
to the verifiler.
Extend btf_kfunc_id_set to handle kfunc polymorphism. Specifically,
a list of kfuncs, "hidden_set", and a new method "remap" is added to
btf_kfunc_id_set. kfuncs in hidden_set do not have BTF_SET8_KFUNCS flag,
and are not exposed in vmlinux.h. The remap method is used to inspect
the calling context, and when necessary, remap the user visible kfuncs
(for example, bpf_dynptr_from_skb), to its hidden version (for example,
bpf_dynptr_from_skb_rdonly).
The verifier calls in these remap logic via the new btf_kfunc_id_remap()
API, and picks the right kfuncs for the context.
Signed-off-by: Song Liu <song@kernel.org>
---
include/linux/btf.h | 20 +++++++
include/linux/btf_ids.h | 3 ++
kernel/bpf/btf.c | 117 ++++++++++++++++++++++++++++++++++------
kernel/bpf/verifier.c | 6 ++-
4 files changed, 127 insertions(+), 19 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 2a08a2b55592..065c374c4372 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -114,11 +114,23 @@ struct btf_id_set;
struct bpf_prog;
typedef int (*btf_kfunc_filter_t)(const struct bpf_prog *prog, u32 kfunc_id);
+typedef u32 (*btf_kfunc_remap_t)(const struct bpf_prog *prog, u32 kfunc_id);
struct btf_kfunc_id_set {
struct module *owner;
struct btf_id_set8 *set;
+
+ /* *hidden_set* contains kfuncs that are not exposed as kfunc in
+ * vmlinux.h. These kfuncs are usually a variation of a kfunc
+ * in *set*.
+ */
+ struct btf_id_set8 *hidden_set;
btf_kfunc_filter_t filter;
+
+ /* *remap* method remaps kfuncs in *set* to proper version in
+ * *hidden_set*.
+ */
+ btf_kfunc_remap_t remap;
};
struct btf_id_dtor_kfunc {
@@ -575,6 +587,8 @@ u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id,
int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
const struct btf_kfunc_id_set *s);
int register_btf_fmodret_id_set(const struct btf_kfunc_id_set *kset);
+u32 btf_kfunc_id_remap(const struct btf *btf, u32 kfunc_btf_id,
+ const struct bpf_prog *prog);
s32 btf_find_dtor_kfunc(struct btf *btf, u32 btf_id);
int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dtors, u32 add_cnt,
struct module *owner);
@@ -637,6 +651,12 @@ static inline u32 *btf_kfunc_id_set_contains(const struct btf *btf,
{
return NULL;
}
+static inline u32 btf_kfunc_id_remap(const struct btf *btf, u32 kfunc_btf_id,
+ const struct bpf_prog *prog)
+{
+ return kfunc_btf_id;
+}
+
static inline int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
const struct btf_kfunc_id_set *s)
{
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index 139bdececdcf..e95b72fbba48 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -212,6 +212,9 @@ extern struct btf_id_set8 name;
#define BTF_KFUNCS_START(name) \
__BTF_SET8_START(name, local, BTF_SET8_KFUNCS)
+#define BTF_HIDDEN_KFUNCS_START(name) \
+__BTF_SET8_START(name, local, 0)
+
#define BTF_KFUNCS_END(name) \
BTF_SET8_END(name)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 8396ce1d0fba..0ffe99205e9c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -226,6 +226,7 @@ enum {
BTF_KFUNC_SET_MAX_CNT = 256,
BTF_DTOR_KFUNC_MAX_CNT = 256,
BTF_KFUNC_FILTER_MAX_CNT = 16,
+ BTF_KFUNC_REMAP_MAX_CNT = 16,
};
struct btf_kfunc_hook_filter {
@@ -233,9 +234,15 @@ struct btf_kfunc_hook_filter {
u32 nr_filters;
};
+struct btf_kfunc_hook_remap {
+ btf_kfunc_remap_t remaps[BTF_KFUNC_REMAP_MAX_CNT];
+ u32 nr_remaps;
+};
+
struct btf_kfunc_set_tab {
struct btf_id_set8 *sets[BTF_KFUNC_HOOK_MAX];
struct btf_kfunc_hook_filter hook_filters[BTF_KFUNC_HOOK_MAX];
+ struct btf_kfunc_hook_remap hook_remaps[BTF_KFUNC_HOOK_MAX];
};
struct btf_id_dtor_kfunc_tab {
@@ -8372,16 +8379,35 @@ static int btf_check_kfunc_protos(struct btf *btf, u32 func_id, u32 func_flags)
/* Kernel Function (kfunc) BTF ID set registration API */
+static void btf_add_kfunc_to_set(struct btf *btf, struct btf_id_set8 *set,
+ struct btf_id_set8 *add_set)
+{
+ u32 i;
+
+ if (!add_set)
+ return;
+ /* Concatenate the two sets */
+ memcpy(set->pairs + set->cnt, add_set->pairs, add_set->cnt * sizeof(set->pairs[0]));
+ /* Now that the set is copied, update with relocated BTF ids */
+ for (i = set->cnt; i < set->cnt + add_set->cnt; i++)
+ set->pairs[i].id = btf_relocate_id(btf, set->pairs[i].id);
+
+ set->cnt += add_set->cnt;
+
+ sort(set->pairs, set->cnt, sizeof(set->pairs[0]), btf_id_cmp_func, NULL);
+}
+
static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
const struct btf_kfunc_id_set *kset)
{
struct btf_kfunc_hook_filter *hook_filter;
- struct btf_id_set8 *add_set = kset->set;
+ struct btf_kfunc_hook_remap *hook_remap;
bool vmlinux_set = !btf_is_module(btf);
bool add_filter = !!kset->filter;
+ bool add_remap = !!kset->remap;
struct btf_kfunc_set_tab *tab;
struct btf_id_set8 *set;
- u32 set_cnt, i;
+ u32 set_cnt, add_cnt, i;
int ret;
if (hook >= BTF_KFUNC_HOOK_MAX) {
@@ -8389,14 +8415,16 @@ static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
goto end;
}
- if (!add_set->cnt)
+ add_cnt = kset->set->cnt;
+ if (kset->hidden_set)
+ add_cnt += kset->hidden_set->cnt;
+
+ if (!add_cnt)
return 0;
tab = btf->kfunc_set_tab;
if (tab && add_filter) {
- u32 i;
-
hook_filter = &tab->hook_filters[hook];
for (i = 0; i < hook_filter->nr_filters; i++) {
if (hook_filter->filters[i] == kset->filter) {
@@ -8411,6 +8439,21 @@ static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
}
}
+ if (tab && add_remap) {
+ hook_remap = &tab->hook_remaps[hook];
+ for (i = 0; i < hook_remap->nr_remaps; i++) {
+ if (hook_remap->remaps[i] == kset->remap) {
+ add_remap = false;
+ break;
+ }
+ }
+
+ if (add_remap && hook_remap->nr_remaps == BTF_KFUNC_REMAP_MAX_CNT) {
+ ret = -E2BIG;
+ goto end;
+ }
+ }
+
if (!tab) {
tab = kzalloc(sizeof(*tab), GFP_KERNEL | __GFP_NOWARN);
if (!tab)
@@ -8439,19 +8482,19 @@ static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
*/
set_cnt = set ? set->cnt : 0;
- if (set_cnt > U32_MAX - add_set->cnt) {
+ if (set_cnt > U32_MAX - add_cnt) {
ret = -EOVERFLOW;
goto end;
}
- if (set_cnt + add_set->cnt > BTF_KFUNC_SET_MAX_CNT) {
+ if (set_cnt + add_cnt > BTF_KFUNC_SET_MAX_CNT) {
ret = -E2BIG;
goto end;
}
/* Grow set */
set = krealloc(tab->sets[hook],
- offsetof(struct btf_id_set8, pairs[set_cnt + add_set->cnt]),
+ offsetof(struct btf_id_set8, pairs[set_cnt + add_cnt]),
GFP_KERNEL | __GFP_NOWARN);
if (!set) {
ret = -ENOMEM;
@@ -8463,20 +8506,18 @@ static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
set->cnt = 0;
tab->sets[hook] = set;
- /* Concatenate the two sets */
- memcpy(set->pairs + set->cnt, add_set->pairs, add_set->cnt * sizeof(set->pairs[0]));
- /* Now that the set is copied, update with relocated BTF ids */
- for (i = set->cnt; i < set->cnt + add_set->cnt; i++)
- set->pairs[i].id = btf_relocate_id(btf, set->pairs[i].id);
-
- set->cnt += add_set->cnt;
-
- sort(set->pairs, set->cnt, sizeof(set->pairs[0]), btf_id_cmp_func, NULL);
+ btf_add_kfunc_to_set(btf, set, kset->set);
+ btf_add_kfunc_to_set(btf, set, kset->hidden_set);
if (add_filter) {
hook_filter = &tab->hook_filters[hook];
hook_filter->filters[hook_filter->nr_filters++] = kset->filter;
}
+
+ if (add_remap) {
+ hook_remap = &tab->hook_remaps[hook];
+ hook_remap->remaps[hook_remap->nr_remaps++] = kset->remap;
+ }
return 0;
end:
btf_free_kfunc_set_tab(btf);
@@ -8511,6 +8552,28 @@ static u32 *__btf_kfunc_id_set_contains(const struct btf *btf,
return id + 1;
}
+static u32 __btf_kfunc_id_remap(const struct btf *btf,
+ enum btf_kfunc_hook hook,
+ u32 kfunc_btf_id,
+ const struct bpf_prog *prog)
+{
+ struct btf_kfunc_hook_remap *hook_remap;
+ u32 i, remap_id = 0;
+
+ if (hook >= BTF_KFUNC_HOOK_MAX)
+ return 0;
+ if (!btf->kfunc_set_tab)
+ return 0;
+ hook_remap = &btf->kfunc_set_tab->hook_remaps[hook];
+
+ for (i = 0; i < hook_remap->nr_remaps; i++) {
+ remap_id = hook_remap->remaps[i](prog, kfunc_btf_id);
+ if (remap_id)
+ break;
+ }
+ return remap_id;
+}
+
static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
{
switch (prog_type) {
@@ -8579,6 +8642,26 @@ u32 *btf_kfunc_id_set_contains(const struct btf *btf,
return __btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id, prog);
}
+/* Reference to the module (obtained using btf_try_get_module)
+ * corresponding to the struct btf *MUST* be held when calling this
+ * function from the verifier
+ */
+u32 btf_kfunc_id_remap(const struct btf *btf, u32 kfunc_btf_id,
+ const struct bpf_prog *prog)
+{
+ enum bpf_prog_type prog_type = resolve_prog_type(prog);
+ enum btf_kfunc_hook hook;
+ u32 remap_id;
+
+ remap_id = __btf_kfunc_id_remap(btf, BTF_KFUNC_HOOK_COMMON, kfunc_btf_id, prog);
+ if (remap_id)
+ return remap_id;
+
+ hook = bpf_prog_type_to_kfunc_hook(prog_type);
+ remap_id = __btf_kfunc_id_remap(btf, hook, kfunc_btf_id, prog);
+ return remap_id ?: kfunc_btf_id;
+}
+
u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id,
const struct bpf_prog *prog)
{
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b8ca227c78af..c321fd25fca3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3029,13 +3029,14 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
return btf_vmlinux ?: ERR_PTR(-ENOENT);
}
-static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
+static int add_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, s16 offset)
{
const struct btf_type *func, *func_proto;
struct bpf_kfunc_btf_tab *btf_tab;
struct bpf_kfunc_desc_tab *tab;
struct bpf_prog_aux *prog_aux;
struct bpf_kfunc_desc *desc;
+ u32 func_id = insn->imm;
const char *func_name;
struct btf *desc_btf;
unsigned long call_imm;
@@ -3094,6 +3095,7 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
return PTR_ERR(desc_btf);
}
+ func_id = insn->imm = btf_kfunc_id_remap(desc_btf, insn->imm, env->prog);
if (find_kfunc_desc(env->prog, func_id, offset))
return 0;
@@ -3227,7 +3229,7 @@ static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
if (bpf_pseudo_func(insn) || bpf_pseudo_call(insn))
ret = add_subprog(env, i + insn->imm + 1);
else
- ret = add_kfunc_call(env, insn->imm, insn->off);
+ ret = add_kfunc_call(env, insn, insn->off);
if (ret < 0)
return ret;
--
2.43.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
2025-01-08 22:51 [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs Song Liu
` (3 preceding siblings ...)
2025-01-08 22:51 ` [PATCH v8 bpf-next 4/7] bpf: Extend btf_kfunc_id_set to handle kfunc polymorphism Song Liu
@ 2025-01-08 22:51 ` Song Liu
2025-01-09 23:55 ` kernel test robot
` (2 more replies)
2025-01-08 22:51 ` [PATCH v8 bpf-next 6/7] bpf: fs/xattr: Add BPF kfuncs to set and remove xattrs Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 7/7] selftests/bpf: Test kfuncs that set and remove xattr from BPF programs Song Liu
6 siblings, 3 replies; 14+ messages in thread
From: Song Liu @ 2025-01-08 22:51 UTC (permalink / raw)
To: bpf, linux-kernel, linux-security-module
Cc: kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor, Song Liu
btf_kfunc_id_set.remap can pick proper version of a kfunc for the calling
context. Use this logic to select bpf_dynptr_from_skb or
bpf_dynptr_from_skb_rdonly. This will make the verifier simpler.
Unfortunately, btf_kfunc_id_set.remap cannot cover the DYNPTR_TYPE_SKB
logic in check_kfunc_args(). This can be addressed later.
Signed-off-by: Song Liu <song@kernel.org>
---
kernel/bpf/verifier.c | 25 ++++++----------------
net/core/filter.c | 49 +++++++++++++++++++++++++++++++++++++++----
2 files changed, 51 insertions(+), 23 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c321fd25fca3..95b0847191fe 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11677,6 +11677,7 @@ enum special_kfunc_type {
KF_bpf_rbtree_add_impl,
KF_bpf_rbtree_first,
KF_bpf_dynptr_from_skb,
+ KF_bpf_dynptr_from_skb_rdonly,
KF_bpf_dynptr_from_xdp,
KF_bpf_dynptr_slice,
KF_bpf_dynptr_slice_rdwr,
@@ -11712,6 +11713,7 @@ BTF_ID(func, bpf_rbtree_add_impl)
BTF_ID(func, bpf_rbtree_first)
#ifdef CONFIG_NET
BTF_ID(func, bpf_dynptr_from_skb)
+BTF_ID(func, bpf_dynptr_from_skb_rdonly)
BTF_ID(func, bpf_dynptr_from_xdp)
#endif
BTF_ID(func, bpf_dynptr_slice)
@@ -11743,10 +11745,12 @@ BTF_ID(func, bpf_rbtree_add_impl)
BTF_ID(func, bpf_rbtree_first)
#ifdef CONFIG_NET
BTF_ID(func, bpf_dynptr_from_skb)
+BTF_ID(func, bpf_dynptr_from_skb_rdonly)
BTF_ID(func, bpf_dynptr_from_xdp)
#else
BTF_ID_UNUSED
BTF_ID_UNUSED
+BTF_ID_UNUSED
#endif
BTF_ID(func, bpf_dynptr_slice)
BTF_ID(func, bpf_dynptr_slice_rdwr)
@@ -12668,7 +12672,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
if (is_kfunc_arg_uninit(btf, &args[i]))
dynptr_arg_type |= MEM_UNINIT;
- if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) {
+ if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb] ||
+ meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb_rdonly]) {
dynptr_arg_type |= DYNPTR_TYPE_SKB;
} else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_xdp]) {
dynptr_arg_type |= DYNPTR_TYPE_XDP;
@@ -20821,9 +20826,7 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
u32 func_id, u16 offset, unsigned long *addr)
{
struct bpf_prog *prog = env->prog;
- bool seen_direct_write;
void *xdp_kfunc;
- bool is_rdonly;
if (bpf_dev_bound_kfunc_id(func_id)) {
xdp_kfunc = bpf_dev_bound_resolve_kfunc(prog, func_id);
@@ -20833,22 +20836,6 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
}
/* fallback to default kfunc when not supported by netdev */
}
-
- if (offset)
- return;
-
- if (func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) {
- seen_direct_write = env->seen_direct_write;
- is_rdonly = !may_access_direct_pkt_data(env, NULL, BPF_WRITE);
-
- if (is_rdonly)
- *addr = (unsigned long)bpf_dynptr_from_skb_rdonly;
-
- /* restore env->seen_direct_write to its original value, since
- * may_access_direct_pkt_data mutates it
- */
- env->seen_direct_write = seen_direct_write;
- }
}
static void __fixup_collection_insert_kfunc(struct bpf_insn_aux_data *insn_aux,
diff --git a/net/core/filter.c b/net/core/filter.c
index 21131ec25f24..f12bcc1b21d1 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -12047,10 +12047,8 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk,
#endif
}
-__bpf_kfunc_end_defs();
-
-int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
- struct bpf_dynptr *ptr__uninit)
+__bpf_kfunc int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
+ struct bpf_dynptr *ptr__uninit)
{
struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit;
int err;
@@ -12064,10 +12062,16 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
return 0;
}
+__bpf_kfunc_end_defs();
+
BTF_KFUNCS_START(bpf_kfunc_check_set_skb)
BTF_ID_FLAGS(func, bpf_dynptr_from_skb, KF_TRUSTED_ARGS)
BTF_KFUNCS_END(bpf_kfunc_check_set_skb)
+BTF_HIDDEN_KFUNCS_START(bpf_kfunc_check_hidden_set_skb)
+BTF_ID_FLAGS(func, bpf_dynptr_from_skb_rdonly, KF_TRUSTED_ARGS)
+BTF_KFUNCS_END(bpf_kfunc_check_hidden_set_skb)
+
BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
BTF_ID_FLAGS(func, bpf_dynptr_from_xdp)
BTF_KFUNCS_END(bpf_kfunc_check_set_xdp)
@@ -12080,9 +12084,46 @@ BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
BTF_ID_FLAGS(func, bpf_sk_assign_tcp_reqsk, KF_TRUSTED_ARGS)
BTF_KFUNCS_END(bpf_kfunc_check_set_tcp_reqsk)
+BTF_ID_LIST(bpf_dynptr_from_skb_list)
+BTF_ID(func, bpf_dynptr_from_skb)
+BTF_ID(func, bpf_dynptr_from_skb_rdonly)
+
+static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id)
+{
+ if (kfunc_id != bpf_dynptr_from_skb_list[0])
+ return 0;
+
+ switch (resolve_prog_type(prog)) {
+ /* Program types only with direct read access go here! */
+ case BPF_PROG_TYPE_LWT_IN:
+ case BPF_PROG_TYPE_LWT_OUT:
+ case BPF_PROG_TYPE_LWT_SEG6LOCAL:
+ case BPF_PROG_TYPE_SK_REUSEPORT:
+ case BPF_PROG_TYPE_FLOW_DISSECTOR:
+ case BPF_PROG_TYPE_CGROUP_SKB:
+ return bpf_dynptr_from_skb_list[1];
+
+ /* Program types with direct read + write access go here! */
+ case BPF_PROG_TYPE_SCHED_CLS:
+ case BPF_PROG_TYPE_SCHED_ACT:
+ case BPF_PROG_TYPE_XDP:
+ case BPF_PROG_TYPE_LWT_XMIT:
+ case BPF_PROG_TYPE_SK_SKB:
+ case BPF_PROG_TYPE_SK_MSG:
+ case BPF_PROG_TYPE_CGROUP_SOCKOPT:
+ return kfunc_id;
+
+ default:
+ break;
+ }
+ return bpf_dynptr_from_skb_list[1];
+}
+
static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
.owner = THIS_MODULE,
.set = &bpf_kfunc_check_set_skb,
+ .hidden_set = &bpf_kfunc_check_hidden_set_skb,
+ .remap = &bpf_kfunc_set_skb_remap,
};
static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
--
2.43.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v8 bpf-next 6/7] bpf: fs/xattr: Add BPF kfuncs to set and remove xattrs
2025-01-08 22:51 [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs Song Liu
` (4 preceding siblings ...)
2025-01-08 22:51 ` [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb Song Liu
@ 2025-01-08 22:51 ` Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 7/7] selftests/bpf: Test kfuncs that set and remove xattr from BPF programs Song Liu
6 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2025-01-08 22:51 UTC (permalink / raw)
To: bpf, linux-kernel, linux-security-module
Cc: kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor, Song Liu
Add the following kfuncs to set and remove xattrs from BPF programs:
bpf_set_dentry_xattr
bpf_remove_dentry_xattr
bpf_set_dentry_xattr_locked
bpf_remove_dentry_xattr_locked
The _locked version of these kfuncs are called from hooks where
dentry->d_inode is already locked. Instead of requiring the user
to know which version of the kfuncs to use, the verifier will pick
the proper kfunc based on the calling hook.
Signed-off-by: Song Liu <song@kernel.org>
---
fs/bpf_fs_kfuncs.c | 227 +++++++++++++++++++++++++++++++++++++++-
include/linux/bpf_lsm.h | 2 +
2 files changed, 227 insertions(+), 2 deletions(-)
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 8a65184c8c2c..f68c83bfb93f 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -2,10 +2,12 @@
/* Copyright (c) 2024 Google LLC. */
#include <linux/bpf.h>
+#include <linux/bpf_lsm.h>
#include <linux/btf.h>
#include <linux/btf_ids.h>
#include <linux/dcache.h>
#include <linux/fs.h>
+#include <linux/fsnotify.h>
#include <linux/file.h>
#include <linux/mm.h>
#include <linux/xattr.h>
@@ -161,6 +163,164 @@ __bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,
return bpf_get_dentry_xattr(dentry, name__str, value_p);
}
+static int bpf_xattr_write_permission(const char *name, struct inode *inode)
+{
+ if (WARN_ON(!inode))
+ return -EINVAL;
+
+ /* Only allow setting and removing security.bpf. xattrs */
+ if (!match_security_bpf_prefix(name))
+ return -EPERM;
+
+ return inode_permission(&nop_mnt_idmap, inode, MAY_WRITE);
+}
+
+static int __bpf_set_dentry_xattr(struct dentry *dentry, const char *name,
+ const struct bpf_dynptr *value_p, int flags, bool lock_inode)
+{
+ struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p;
+ struct inode *inode = d_inode(dentry);
+ const void *value;
+ u32 value_len;
+ int ret;
+
+ value_len = __bpf_dynptr_size(value_ptr);
+ value = __bpf_dynptr_data(value_ptr, value_len);
+ if (!value)
+ return -EINVAL;
+
+ if (lock_inode)
+ inode_lock(inode);
+
+ ret = bpf_xattr_write_permission(name, inode);
+ if (ret)
+ goto out;
+
+ ret = __vfs_setxattr(&nop_mnt_idmap, dentry, inode, name,
+ value, value_len, flags);
+ if (!ret) {
+ fsnotify_xattr(dentry);
+
+ /* This xattr is set by BPF LSM, so we do not call
+ * security_inode_post_setxattr. This is the same as
+ * security_inode_setsecurity().
+ */
+ }
+out:
+ if (lock_inode)
+ inode_unlock(inode);
+ return ret;
+}
+
+/**
+ * bpf_set_dentry_xattr - set a xattr of a dentry
+ * @dentry: dentry to get xattr from
+ * @name__str: name of the xattr
+ * @value_p: xattr value
+ * @flags: flags to pass into filesystem operations
+ *
+ * Set xattr *name__str* of *dentry* to the value in *value_ptr*.
+ *
+ * For security reasons, only *name__str* with prefix "security.bpf."
+ * is allowed.
+ *
+ * The caller has not locked dentry->d_inode.
+ *
+ * Return: 0 on success, a negative value on error.
+ */
+__bpf_kfunc int bpf_set_dentry_xattr(struct dentry *dentry, const char *name__str,
+ const struct bpf_dynptr *value_p, int flags)
+{
+ return __bpf_set_dentry_xattr(dentry, name__str, value_p, flags, true);
+}
+
+/**
+ * bpf_set_dentry_xattr_locked - set a xattr of a dentry
+ * @dentry: dentry to get xattr from
+ * @name__str: name of the xattr
+ * @value_p: xattr value
+ * @flags: flags to pass into filesystem operations
+ *
+ * Set xattr *name__str* of *dentry* to the value in *value_ptr*.
+ *
+ * For security reasons, only *name__str* with prefix "security.bpf."
+ * is allowed.
+ *
+ * The caller already locked dentry->d_inode.
+ *
+ * Return: 0 on success, a negative value on error.
+ */
+__bpf_kfunc int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
+ const struct bpf_dynptr *value_p, int flags)
+{
+ return __bpf_set_dentry_xattr(dentry, name__str, value_p, flags, false);
+}
+
+static int __bpf_remove_dentry_xattr(struct dentry *dentry, const char *name__str,
+ bool lock_inode)
+{
+ struct inode *inode = d_inode(dentry);
+ int ret;
+
+ if (lock_inode)
+ inode_lock(inode);
+
+ ret = bpf_xattr_write_permission(name__str, inode);
+ if (ret)
+ goto out;
+
+ ret = __vfs_removexattr(&nop_mnt_idmap, dentry, name__str);
+ if (!ret) {
+ fsnotify_xattr(dentry);
+
+ /* This xattr is removed by BPF LSM, so we do not call
+ * security_inode_post_removexattr.
+ */
+ }
+out:
+ if (lock_inode)
+ inode_unlock(inode);
+ return ret;
+}
+
+/**
+ * bpf_remove_dentry_xattr - remove a xattr of a dentry
+ * @dentry: dentry to get xattr from
+ * @name__str: name of the xattr
+ *
+ * Rmove xattr *name__str* of *dentry*.
+ *
+ * For security reasons, only *name__str* with prefix "security.bpf."
+ * is allowed.
+ *
+ * The caller has not locked dentry->d_inode.
+ *
+ * Return: 0 on success, a negative value on error.
+ */
+__bpf_kfunc int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name__str)
+{
+ return __bpf_remove_dentry_xattr(dentry, name__str, true);
+}
+
+/**
+ * bpf_remove_dentry_xattr_locked - remove a xattr of a dentry
+ * @dentry: dentry to get xattr from
+ * @name__str: name of the xattr
+ *
+ * Rmove xattr *name__str* of *dentry*.
+ *
+ * For security reasons, only *name__str* with prefix "security.bpf."
+ * is allowed.
+ *
+ * The caller already locked dentry->d_inode.
+ *
+ * Return: 0 on success, a negative value on error.
+ */
+__bpf_kfunc int bpf_remove_dentry_xattr_locked(struct dentry *dentry, const char *name__str)
+{
+ return __bpf_remove_dentry_xattr(dentry, name__str, false);
+}
+
__bpf_kfunc_end_defs();
BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
@@ -170,20 +330,83 @@ BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_path_d_path, KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
+BTF_HIDDEN_KFUNCS_START(bpf_fs_kfunc_hidden_set_ids)
+BTF_ID_FLAGS(func, bpf_set_dentry_xattr_locked, KF_SLEEPABLE | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_remove_dentry_xattr_locked, KF_SLEEPABLE | KF_TRUSTED_ARGS)
+BTF_KFUNCS_END(bpf_fs_kfunc_hidden_set_ids)
+
static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
{
- if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) ||
- prog->type == BPF_PROG_TYPE_LSM)
+ if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) &&
+ !btf_id_set8_contains(&bpf_fs_kfunc_hidden_set_ids, kfunc_id))
+ return 0;
+ if (prog->type == BPF_PROG_TYPE_LSM)
return 0;
return -EACCES;
}
+/* bpf_[set|remove]_dentry_xattr.* hooks have KF_TRUSTED_ARGS and
+ * KF_SLEEPABLE, so they are only available to sleepable hooks with
+ * dentry arguments.
+ *
+ * Setting and removing xattr requires exclusive lock on dentry->d_inode.
+ * Some hooks already locked d_inode, while some hooks have not locked
+ * d_inode. Therefore, we need different kfuncs for different hooks.
+ * Specifically, hooks in the following list (d_inode_locked_hooks)
+ * should call bpf_[set|remove]_dentry_xattr_locked; while other hooks
+ * should call bpf_[set|remove]_dentry_xattr.
+ */
+BTF_SET_START(d_inode_locked_hooks)
+BTF_ID(func, bpf_lsm_inode_post_removexattr)
+BTF_ID(func, bpf_lsm_inode_post_setattr)
+BTF_ID(func, bpf_lsm_inode_post_setxattr)
+BTF_ID(func, bpf_lsm_inode_removexattr)
+BTF_ID(func, bpf_lsm_inode_rmdir)
+BTF_ID(func, bpf_lsm_inode_setattr)
+BTF_ID(func, bpf_lsm_inode_setxattr)
+BTF_ID(func, bpf_lsm_inode_unlink)
+#ifdef CONFIG_SECURITY_PATH
+BTF_ID(func, bpf_lsm_path_unlink)
+BTF_ID(func, bpf_lsm_path_rmdir)
+#endif /* CONFIG_SECURITY_PATH */
+BTF_SET_END(d_inode_locked_hooks)
+
+static bool bpf_lsm_has_d_inode_locked(const struct bpf_prog *prog)
+{
+ return btf_id_set_contains(&d_inode_locked_hooks, prog->aux->attach_btf_id);
+}
+
+BTF_ID_LIST(not_locked_fs_kfuncs)
+BTF_ID(func, bpf_set_dentry_xattr)
+BTF_ID(func, bpf_remove_dentry_xattr)
+
+BTF_ID_LIST(locked_fs_kfuncs)
+BTF_ID(func, bpf_set_dentry_xattr_locked)
+BTF_ID(func, bpf_remove_dentry_xattr_locked)
+
+static u32 bpf_fs_kfunc_remap(const struct bpf_prog *prog, u32 kfunc_id)
+{
+ if (!bpf_lsm_has_d_inode_locked(prog))
+ return 0;
+
+ if (kfunc_id == not_locked_fs_kfuncs[0])
+ return locked_fs_kfuncs[0];
+ if (kfunc_id == not_locked_fs_kfuncs[1])
+ return locked_fs_kfuncs[1];
+
+ return 0;
+}
+
static const struct btf_kfunc_id_set bpf_fs_kfunc_set = {
.owner = THIS_MODULE,
.set = &bpf_fs_kfunc_set_ids,
+ .hidden_set = &bpf_fs_kfunc_hidden_set_ids,
.filter = bpf_fs_kfuncs_filter,
+ .remap = bpf_fs_kfunc_remap,
};
static int __init bpf_fs_kfuncs_init(void)
diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index aefcd6564251..f4ab0dc1df69 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -48,6 +48,7 @@ void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func)
int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
struct bpf_retval_range *range);
+
#else /* !CONFIG_BPF_LSM */
static inline bool bpf_lsm_is_sleepable_hook(u32 btf_id)
@@ -86,6 +87,7 @@ static inline int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
{
return -EOPNOTSUPP;
}
+
#endif /* CONFIG_BPF_LSM */
#endif /* _LINUX_BPF_LSM_H */
--
2.43.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v8 bpf-next 7/7] selftests/bpf: Test kfuncs that set and remove xattr from BPF programs
2025-01-08 22:51 [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs Song Liu
` (5 preceding siblings ...)
2025-01-08 22:51 ` [PATCH v8 bpf-next 6/7] bpf: fs/xattr: Add BPF kfuncs to set and remove xattrs Song Liu
@ 2025-01-08 22:51 ` Song Liu
6 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2025-01-08 22:51 UTC (permalink / raw)
To: bpf, linux-kernel, linux-security-module
Cc: kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor, Song Liu
Two sets of tests are added to exercise the not _locked and _locked
version of the kfuncs. For both tests, user space accesses xattr
security.bpf.foo on a testfile. The BPF program is triggered by user
space access (on LSM hook inode_[set|get]_xattr) and sets or removes
xattr security.bpf.bar. Then user space then validates that xattr
security.bpf.bar is set or removed as expected.
Note that, in both tests, the BPF programs use the not _locked kfuncs.
The verifier picks the proper kfuncs based on the calling context.
Signed-off-by: Song Liu <song@kernel.org>
---
tools/testing/selftests/bpf/bpf_kfuncs.h | 5 +
.../selftests/bpf/prog_tests/fs_kfuncs.c | 125 ++++++++++++++++
.../bpf/progs/test_set_remove_xattr.c | 133 ++++++++++++++++++
3 files changed, 263 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/test_set_remove_xattr.c
diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h
index 2eb3483f2fb0..8215c9b3115e 100644
--- a/tools/testing/selftests/bpf/bpf_kfuncs.h
+++ b/tools/testing/selftests/bpf/bpf_kfuncs.h
@@ -87,4 +87,9 @@ struct dentry;
*/
extern int bpf_get_dentry_xattr(struct dentry *dentry, const char *name,
struct bpf_dynptr *value_ptr) __ksym __weak;
+
+extern int bpf_set_dentry_xattr(struct dentry *dentry, const char *name__str,
+ const struct bpf_dynptr *value_p, int flags) __ksym __weak;
+extern int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name__str) __ksym __weak;
+
#endif
diff --git a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
index 419f45b56472..43a26ec69a8e 100644
--- a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
+++ b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
@@ -8,6 +8,7 @@
#include <unistd.h>
#include <test_progs.h>
#include "test_get_xattr.skel.h"
+#include "test_set_remove_xattr.skel.h"
#include "test_fsverity.skel.h"
static const char testfile[] = "/tmp/test_progs_fs_kfuncs";
@@ -72,6 +73,127 @@ static void test_get_xattr(const char *name, const char *value, bool allow_acces
remove(testfile);
}
+/* xattr value we will set to security.bpf.foo */
+static const char value_foo[] = "hello";
+
+static void read_and_validate_foo(struct test_set_remove_xattr *skel)
+{
+ char value_out[32];
+ int err;
+
+ err = getxattr(testfile, skel->rodata->xattr_foo, value_out, sizeof(value_out));
+ ASSERT_EQ(err, sizeof(value_foo), "getxattr size foo");
+ ASSERT_EQ(strncmp(value_out, value_foo, sizeof(value_foo)), 0, "strncmp value_foo");
+}
+
+static void set_foo(struct test_set_remove_xattr *skel)
+{
+ ASSERT_OK(setxattr(testfile, skel->rodata->xattr_foo, value_foo, strlen(value_foo) + 1, 0),
+ "setxattr foo");
+}
+
+static void validate_bar_match(struct test_set_remove_xattr *skel)
+{
+ char value_out[32];
+ int err;
+
+ err = getxattr(testfile, skel->rodata->xattr_bar, value_out, sizeof(value_out));
+ ASSERT_EQ(err, sizeof(skel->data->value_bar), "getxattr size bar");
+ ASSERT_EQ(strncmp(value_out, skel->data->value_bar, sizeof(skel->data->value_bar)), 0,
+ "strncmp value_bar");
+}
+
+static void validate_bar_removed(struct test_set_remove_xattr *skel)
+{
+ char value_out[32];
+ int err;
+
+ err = getxattr(testfile, skel->rodata->xattr_bar, value_out, sizeof(value_out));
+ ASSERT_LT(err, 0, "getxattr size bar should fail");
+}
+
+static void test_set_remove_xattr(void)
+{
+ struct test_set_remove_xattr *skel = NULL;
+ int fd = -1, err;
+
+ fd = open(testfile, O_CREAT | O_RDONLY, 0644);
+ if (!ASSERT_GE(fd, 0, "create_file"))
+ return;
+
+ close(fd);
+ fd = -1;
+
+ skel = test_set_remove_xattr__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "test_set_remove_xattr__open_and_load"))
+ return;
+
+ /* Set security.bpf.foo to "hello" */
+ err = setxattr(testfile, skel->rodata->xattr_foo, value_foo, strlen(value_foo) + 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",
+ __func__, errno);
+ test__skip();
+ goto out;
+ }
+
+ if (!ASSERT_OK(err, "setxattr"))
+ goto out;
+
+ skel->bss->monitored_pid = getpid();
+ err = test_set_remove_xattr__attach(skel);
+ if (!ASSERT_OK(err, "test_set_remove_xattr__attach"))
+ goto out;
+
+ /* First, test not _locked version of the kfuncs with getxattr. */
+
+ /* Read security.bpf.foo and trigger test_inode_getxattr. This
+ * bpf program will set security.bpf.bar to "world".
+ */
+ read_and_validate_foo(skel);
+ validate_bar_match(skel);
+
+ /* Read security.bpf.foo and trigger test_inode_getxattr again.
+ * This will remove xattr security.bpf.bar.
+ */
+ read_and_validate_foo(skel);
+ validate_bar_removed(skel);
+
+ ASSERT_TRUE(skel->bss->set_security_bpf_bar_success, "set_security_bpf_bar_success");
+ ASSERT_TRUE(skel->bss->remove_security_bpf_bar_success, "remove_security_bpf_bar_success");
+ ASSERT_TRUE(skel->bss->set_security_selinux_fail, "set_security_selinux_fail");
+ ASSERT_TRUE(skel->bss->remove_security_selinux_fail, "remove_security_selinux_fail");
+
+ /* Second, test _locked version of the kfuncs, with setxattr */
+
+ /* Set security.bpf.foo and trigger test_inode_setxattr. This
+ * bpf program will set security.bpf.bar to "world".
+ */
+ set_foo(skel);
+ validate_bar_match(skel);
+
+ /* Set security.bpf.foo and trigger test_inode_setxattr again.
+ * This will remove xattr security.bpf.bar.
+ */
+ set_foo(skel);
+ validate_bar_removed(skel);
+
+ ASSERT_TRUE(skel->bss->locked_set_security_bpf_bar_success,
+ "locked_set_security_bpf_bar_success");
+ ASSERT_TRUE(skel->bss->locked_remove_security_bpf_bar_success,
+ "locked_remove_security_bpf_bar_success");
+ ASSERT_TRUE(skel->bss->locked_set_security_selinux_fail,
+ "locked_set_security_selinux_fail");
+ ASSERT_TRUE(skel->bss->locked_remove_security_selinux_fail,
+ "locked_remove_security_selinux_fail");
+
+out:
+ close(fd);
+ test_set_remove_xattr__destroy(skel);
+ remove(testfile);
+}
+
#ifndef SHA256_DIGEST_SIZE
#define SHA256_DIGEST_SIZE 32
#endif
@@ -161,6 +283,9 @@ void test_fs_kfuncs(void)
if (test__start_subtest("security_selinux_xattr_error"))
test_get_xattr("security.selinux", "hello", false);
+ if (test__start_subtest("set_remove_xattr"))
+ test_set_remove_xattr();
+
if (test__start_subtest("fsverity"))
test_fsverity();
}
diff --git a/tools/testing/selftests/bpf/progs/test_set_remove_xattr.c b/tools/testing/selftests/bpf/progs/test_set_remove_xattr.c
new file mode 100644
index 000000000000..e49be3cc4a33
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_set_remove_xattr.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+#include "vmlinux.h"
+#include <errno.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_kfuncs.h"
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+__u32 monitored_pid;
+
+const char xattr_foo[] = "security.bpf.foo";
+const char xattr_bar[] = "security.bpf.bar";
+const char xattr_linux[] = "security.selinux";
+char value_bar[] = "world";
+char read_value[32];
+
+bool set_security_bpf_bar_success;
+bool remove_security_bpf_bar_success;
+bool set_security_selinux_fail;
+bool remove_security_selinux_fail;
+
+char name_buf[32];
+
+static inline bool name_match_foo(const char *name)
+{
+ bpf_probe_read_kernel(name_buf, sizeof(name_buf), name);
+
+ return !bpf_strncmp(name_buf, sizeof(xattr_foo), xattr_foo);
+}
+
+/* Test bpf_set_dentry_xattr and bpf_remove_dentry_xattr */
+SEC("lsm.s/inode_getxattr")
+int BPF_PROG(test_inode_getxattr, struct dentry *dentry, char *name)
+{
+ struct bpf_dynptr value_ptr;
+ __u32 pid;
+ int ret;
+
+ pid = bpf_get_current_pid_tgid() >> 32;
+ if (pid != monitored_pid)
+ return 0;
+
+ /* Only do the following for security.bpf.foo */
+ if (!name_match_foo(name))
+ return 0;
+
+ bpf_dynptr_from_mem(read_value, sizeof(read_value), 0, &value_ptr);
+
+ /* read security.bpf.bar */
+ ret = bpf_get_dentry_xattr(dentry, xattr_bar, &value_ptr);
+
+ if (ret < 0) {
+ /* If security.bpf.bar doesn't exist, set it */
+ bpf_dynptr_from_mem(value_bar, sizeof(value_bar), 0, &value_ptr);
+
+ ret = bpf_set_dentry_xattr(dentry, xattr_bar, &value_ptr, 0);
+ if (!ret)
+ set_security_bpf_bar_success = true;
+ ret = bpf_set_dentry_xattr(dentry, xattr_linux, &value_ptr, 0);
+ if (ret)
+ set_security_selinux_fail = true;
+ } else {
+ /* If security.bpf.bar exists, remove it */
+ ret = bpf_remove_dentry_xattr(dentry, xattr_bar);
+ if (!ret)
+ remove_security_bpf_bar_success = true;
+
+ ret = bpf_remove_dentry_xattr(dentry, xattr_linux);
+ if (ret)
+ remove_security_selinux_fail = true;
+ }
+
+ return 0;
+}
+
+bool locked_set_security_bpf_bar_success;
+bool locked_remove_security_bpf_bar_success;
+bool locked_set_security_selinux_fail;
+bool locked_remove_security_selinux_fail;
+
+/* Test bpf_set_dentry_xattr_locked and bpf_remove_dentry_xattr_locked.
+ * It not necessary to differentiate the _locked version and the
+ * not-_locked version in the BPF program. The verifier will fix them up
+ * properly.
+ */
+SEC("lsm.s/inode_setxattr")
+int BPF_PROG(test_inode_setxattr, struct mnt_idmap *idmap,
+ struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags)
+{
+ struct bpf_dynptr value_ptr;
+ __u32 pid;
+ int ret;
+
+ pid = bpf_get_current_pid_tgid() >> 32;
+ if (pid != monitored_pid)
+ return 0;
+
+ /* Only do the following for security.bpf.foo */
+ if (!name_match_foo(name))
+ return 0;
+
+ bpf_dynptr_from_mem(read_value, sizeof(read_value), 0, &value_ptr);
+
+ /* read security.bpf.bar */
+ ret = bpf_get_dentry_xattr(dentry, xattr_bar, &value_ptr);
+
+ if (ret < 0) {
+ /* If security.bpf.bar doesn't exist, set it */
+ bpf_dynptr_from_mem(value_bar, sizeof(value_bar), 0, &value_ptr);
+
+ ret = bpf_set_dentry_xattr(dentry, xattr_bar, &value_ptr, 0);
+ if (!ret)
+ locked_set_security_bpf_bar_success = true;
+ ret = bpf_set_dentry_xattr(dentry, xattr_linux, &value_ptr, 0);
+ if (ret)
+ locked_set_security_selinux_fail = true;
+ } else {
+ /* If security.bpf.bar exists, remove it */
+ ret = bpf_remove_dentry_xattr(dentry, xattr_bar);
+ if (!ret)
+ locked_remove_security_bpf_bar_success = true;
+
+ ret = bpf_remove_dentry_xattr(dentry, xattr_linux);
+ if (ret)
+ locked_remove_security_selinux_fail = true;
+ }
+
+ return 0;
+}
--
2.43.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
2025-01-08 22:51 ` [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb Song Liu
@ 2025-01-09 23:55 ` kernel test robot
2025-01-10 1:08 ` Song Liu
2025-01-10 1:11 ` kernel test robot
2025-01-14 22:37 ` Andrii Nakryiko
2 siblings, 1 reply; 14+ messages in thread
From: kernel test robot @ 2025-01-09 23:55 UTC (permalink / raw)
To: Song Liu, bpf, linux-kernel, linux-security-module
Cc: oe-kbuild-all, kernel-team, andrii, ast, daniel, martin.lau,
kpsingh, mattbobrowski, paul, jmorris, serge, memxor, Song Liu
Hi Song,
kernel test robot noticed the following build errors:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Song-Liu/fs-xattr-bpf-Introduce-security-bpf-xattr-name-prefix/20250109-065503
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20250108225140.3467654-6-song%40kernel.org
patch subject: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
config: i386-buildonly-randconfig-005-20250110 (https://download.01.org/0day-ci/archive/20250110/202501100757.HDb5slrv-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250110/202501100757.HDb5slrv-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501100757.HDb5slrv-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> net/core/filter.c:12071:1: error: return type defaults to 'int' [-Werror=implicit-int]
12071 | BTF_HIDDEN_KFUNCS_START(bpf_kfunc_check_hidden_set_skb)
| ^~~~~~~~~~~~~~~~~~~~~~~
>> net/core/filter.c:12071:1: error: function declaration isn't a prototype [-Werror=strict-prototypes]
In file included from include/linux/btf.h:10,
from include/linux/bpf.h:28,
from include/linux/bpf_verifier.h:7,
from net/core/filter.c:21:
net/core/filter.c: In function 'BTF_HIDDEN_KFUNCS_START':
>> net/core/filter.c:12075:18: error: storage class specified for parameter 'bpf_kfunc_check_set_xdp'
12075 | BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
>> include/linux/btf_ids.h:235:46: error: parameter 'bpf_kfunc_check_set_xdp' is initialized
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~~~~~~~~
net/core/filter.c:12075:1: note: in expansion of macro 'BTF_KFUNCS_START'
12075 | BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
| ^~~~~~~~~~~~~~~~
>> net/core/filter.c:12079:18: error: storage class specified for parameter 'bpf_kfunc_check_set_sock_addr'
12079 | BTF_KFUNCS_START(bpf_kfunc_check_set_sock_addr)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
>> include/linux/btf_ids.h:235:46: error: parameter 'bpf_kfunc_check_set_sock_addr' is initialized
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~~~~~~~~
net/core/filter.c:12079:1: note: in expansion of macro 'BTF_KFUNCS_START'
12079 | BTF_KFUNCS_START(bpf_kfunc_check_set_sock_addr)
| ^~~~~~~~~~~~~~~~
>> net/core/filter.c:12083:18: error: storage class specified for parameter 'bpf_kfunc_check_set_tcp_reqsk'
12083 | BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
>> include/linux/btf_ids.h:235:46: error: parameter 'bpf_kfunc_check_set_tcp_reqsk' is initialized
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~~~~~~~~
net/core/filter.c:12083:1: note: in expansion of macro 'BTF_KFUNCS_START'
12083 | BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
| ^~~~~~~~~~~~~~~~
>> net/core/filter.c:12087:13: error: storage class specified for parameter 'bpf_dynptr_from_skb_list'
12087 | BTF_ID_LIST(bpf_dynptr_from_skb_list)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:223:53: note: in definition of macro 'BTF_ID_LIST'
223 | #define BTF_ID_LIST(name) static u32 __maybe_unused name[64];
| ^~~~
>> net/core/filter.c:12092:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
12092 | {
| ^
>> net/core/filter.c:12122:38: error: storage class specified for parameter 'bpf_kfunc_set_skb'
12122 | static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
| ^~~~~~~~~~~~~~~~~
>> net/core/filter.c:12122:21: error: parameter 'bpf_kfunc_set_skb' is initialized
12122 | static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
| ^~~~~~~~~~~~~~~~
>> net/core/filter.c:12125:24: error: 'bpf_kfunc_check_hidden_set_skb' undeclared (first use in this function); did you mean 'bpf_kfunc_check_set_skb'?
12125 | .hidden_set = &bpf_kfunc_check_hidden_set_skb,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| bpf_kfunc_check_set_skb
net/core/filter.c:12125:24: note: each undeclared identifier is reported only once for each function it appears in
>> net/core/filter.c:12126:19: error: 'bpf_kfunc_set_skb_remap' undeclared (first use in this function); did you mean 'bpf_kfunc_set_skb'?
12126 | .remap = &bpf_kfunc_set_skb_remap,
| ^~~~~~~~~~~~~~~~~~~~~~~
| bpf_kfunc_set_skb
>> net/core/filter.c:12129:38: error: storage class specified for parameter 'bpf_kfunc_set_xdp'
12129 | static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
| ^~~~~~~~~~~~~~~~~
>> net/core/filter.c:12129:21: error: parameter 'bpf_kfunc_set_xdp' is initialized
12129 | static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
| ^~~~~~~~~~~~~~~~
>> net/core/filter.c:12134:38: error: storage class specified for parameter 'bpf_kfunc_set_sock_addr'
12134 | static const struct btf_kfunc_id_set bpf_kfunc_set_sock_addr = {
| ^~~~~~~~~~~~~~~~~~~~~~~
>> net/core/filter.c:12134:21: error: parameter 'bpf_kfunc_set_sock_addr' is initialized
12134 | static const struct btf_kfunc_id_set bpf_kfunc_set_sock_addr = {
| ^~~~~~~~~~~~~~~~
>> net/core/filter.c:12139:38: error: storage class specified for parameter 'bpf_kfunc_set_tcp_reqsk'
12139 | static const struct btf_kfunc_id_set bpf_kfunc_set_tcp_reqsk = {
| ^~~~~~~~~~~~~~~~~~~~~~~
>> net/core/filter.c:12139:21: error: parameter 'bpf_kfunc_set_tcp_reqsk' is initialized
12139 | static const struct btf_kfunc_id_set bpf_kfunc_set_tcp_reqsk = {
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12145:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
12145 | {
| ^
In file included from <command-line>:
include/linux/compiler.h:189:45: error: storage class specified for parameter '__UNIQUE_ID___addressable_bpf_kfunc_init1505'
189 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
| ^~~~~~~~~~~~
include/linux/compiler_types.h:83:23: note: in definition of macro '___PASTE'
83 | #define ___PASTE(a,b) a##b
| ^
include/linux/compiler.h:189:29: note: in expansion of macro '__PASTE'
189 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
| ^~~~~~~
include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^~~~~~~~
include/linux/compiler.h:189:37: note: in expansion of macro '__PASTE'
189 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
| ^~~~~~~
include/linux/compiler.h:227:9: note: in expansion of macro '__UNIQUE_ID'
227 | __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
| ^~~~~~~~~~~
include/linux/compiler.h:229:9: note: in expansion of macro '___ADDRESSABLE'
229 | ___ADDRESSABLE(sym, __section(".discard.addressable"))
| ^~~~~~~~~~~~~~
include/linux/init.h:256:9: note: in expansion of macro '__ADDRESSABLE'
256 | __ADDRESSABLE(fn)
| ^~~~~~~~~~~~~
include/linux/init.h:261:9: note: in expansion of macro '__define_initcall_stub'
261 | __define_initcall_stub(__stub, fn) \
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/init.h:274:9: note: in expansion of macro '____define_initcall'
274 | ____define_initcall(fn, \
| ^~~~~~~~~~~~~~~~~~~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12164:1: note: in expansion of macro 'late_initcall'
12164 | late_initcall(bpf_kfunc_init);
| ^~~~~~~~~~~~~
net/core/filter.c:12164:1: error: parameter '__UNIQUE_ID___addressable_bpf_kfunc_init1505' is initialized
net/core/filter.c:12164:1: warning: 'used' attribute ignored [-Wattributes]
include/linux/compiler.h:189:45: error: section attribute not allowed for '__UNIQUE_ID___addressable_bpf_kfunc_init1505'
189 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
| ^~~~~~~~~~~~
include/linux/compiler_types.h:83:23: note: in definition of macro '___PASTE'
83 | #define ___PASTE(a,b) a##b
| ^
include/linux/compiler.h:189:29: note: in expansion of macro '__PASTE'
189 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
| ^~~~~~~
include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^~~~~~~~
include/linux/compiler.h:189:37: note: in expansion of macro '__PASTE'
189 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
| ^~~~~~~
include/linux/compiler.h:227:9: note: in expansion of macro '__UNIQUE_ID'
227 | __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
| ^~~~~~~~~~~
include/linux/compiler.h:229:9: note: in expansion of macro '___ADDRESSABLE'
229 | ___ADDRESSABLE(sym, __section(".discard.addressable"))
| ^~~~~~~~~~~~~~
include/linux/init.h:256:9: note: in expansion of macro '__ADDRESSABLE'
256 | __ADDRESSABLE(fn)
| ^~~~~~~~~~~~~
include/linux/init.h:261:9: note: in expansion of macro '__define_initcall_stub'
261 | __define_initcall_stub(__stub, fn) \
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/init.h:274:9: note: in expansion of macro '____define_initcall'
274 | ____define_initcall(fn, \
| ^~~~~~~~~~~~~~~~~~~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12164:1: note: in expansion of macro 'late_initcall'
12164 | late_initcall(bpf_kfunc_init);
| ^~~~~~~~~~~~~
In file included from arch/x86/include/asm/atomic.h:5,
from include/linux/atomic.h:7,
from net/core/filter.c:20:
net/core/filter.c:12164:15: error: 'bpf_kfunc_init' undeclared (first use in this function); did you mean 'bpf_func_info'?
12164 | late_initcall(bpf_kfunc_init);
| ^~~~~~~~~~~~~~
include/linux/compiler.h:227:72: note: in definition of macro '___ADDRESSABLE'
227 | __UNIQUE_ID(__PASTE(__addressable_,sym)) = (void *)(uintptr_t)&sym;
vim +/int +12071 net/core/filter.c
12070
12071 BTF_HIDDEN_KFUNCS_START(bpf_kfunc_check_hidden_set_skb)
12072 BTF_ID_FLAGS(func, bpf_dynptr_from_skb_rdonly, KF_TRUSTED_ARGS)
12073 BTF_KFUNCS_END(bpf_kfunc_check_hidden_set_skb)
12074
12075 BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
12076 BTF_ID_FLAGS(func, bpf_dynptr_from_xdp)
12077 BTF_KFUNCS_END(bpf_kfunc_check_set_xdp)
12078
12079 BTF_KFUNCS_START(bpf_kfunc_check_set_sock_addr)
12080 BTF_ID_FLAGS(func, bpf_sock_addr_set_sun_path)
12081 BTF_KFUNCS_END(bpf_kfunc_check_set_sock_addr)
12082
12083 BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
12084 BTF_ID_FLAGS(func, bpf_sk_assign_tcp_reqsk, KF_TRUSTED_ARGS)
12085 BTF_KFUNCS_END(bpf_kfunc_check_set_tcp_reqsk)
12086
12087 BTF_ID_LIST(bpf_dynptr_from_skb_list)
12088 BTF_ID(func, bpf_dynptr_from_skb)
12089 BTF_ID(func, bpf_dynptr_from_skb_rdonly)
12090
12091 static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id)
12092 {
12093 if (kfunc_id != bpf_dynptr_from_skb_list[0])
12094 return 0;
12095
12096 switch (resolve_prog_type(prog)) {
12097 /* Program types only with direct read access go here! */
12098 case BPF_PROG_TYPE_LWT_IN:
12099 case BPF_PROG_TYPE_LWT_OUT:
12100 case BPF_PROG_TYPE_LWT_SEG6LOCAL:
12101 case BPF_PROG_TYPE_SK_REUSEPORT:
12102 case BPF_PROG_TYPE_FLOW_DISSECTOR:
12103 case BPF_PROG_TYPE_CGROUP_SKB:
12104 return bpf_dynptr_from_skb_list[1];
12105
12106 /* Program types with direct read + write access go here! */
12107 case BPF_PROG_TYPE_SCHED_CLS:
12108 case BPF_PROG_TYPE_SCHED_ACT:
12109 case BPF_PROG_TYPE_XDP:
12110 case BPF_PROG_TYPE_LWT_XMIT:
12111 case BPF_PROG_TYPE_SK_SKB:
12112 case BPF_PROG_TYPE_SK_MSG:
12113 case BPF_PROG_TYPE_CGROUP_SOCKOPT:
12114 return kfunc_id;
12115
12116 default:
12117 break;
12118 }
12119 return bpf_dynptr_from_skb_list[1];
12120 }
12121
12122 static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
12123 .owner = THIS_MODULE,
12124 .set = &bpf_kfunc_check_set_skb,
12125 .hidden_set = &bpf_kfunc_check_hidden_set_skb,
12126 .remap = &bpf_kfunc_set_skb_remap,
12127 };
12128
12129 static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
12130 .owner = THIS_MODULE,
12131 .set = &bpf_kfunc_check_set_xdp,
12132 };
12133
12134 static const struct btf_kfunc_id_set bpf_kfunc_set_sock_addr = {
12135 .owner = THIS_MODULE,
12136 .set = &bpf_kfunc_check_set_sock_addr,
12137 };
12138
12139 static const struct btf_kfunc_id_set bpf_kfunc_set_tcp_reqsk = {
12140 .owner = THIS_MODULE,
12141 .set = &bpf_kfunc_check_set_tcp_reqsk,
12142 };
12143
12144 static int __init bpf_kfunc_init(void)
12145 {
12146 int ret;
12147
12148 ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_kfunc_set_skb);
12149 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_ACT, &bpf_kfunc_set_skb);
12150 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SK_SKB, &bpf_kfunc_set_skb);
12151 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SOCKET_FILTER, &bpf_kfunc_set_skb);
12152 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SKB, &bpf_kfunc_set_skb);
12153 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_OUT, &bpf_kfunc_set_skb);
12154 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_IN, &bpf_kfunc_set_skb);
12155 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_XMIT, &bpf_kfunc_set_skb);
12156 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_SEG6LOCAL, &bpf_kfunc_set_skb);
12157 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_NETFILTER, &bpf_kfunc_set_skb);
12158 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_kfunc_set_skb);
12159 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &bpf_kfunc_set_xdp);
12160 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
12161 &bpf_kfunc_set_sock_addr);
12162 return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_kfunc_set_tcp_reqsk);
12163 }
12164 late_initcall(bpf_kfunc_init);
12165
12166 __bpf_kfunc_start_defs();
12167
12168 /* bpf_sock_destroy: Destroy the given socket with ECONNABORTED error code.
12169 *
12170 * The function expects a non-NULL pointer to a socket, and invokes the
12171 * protocol specific socket destroy handlers.
12172 *
12173 * The helper can only be called from BPF contexts that have acquired the socket
12174 * locks.
12175 *
12176 * Parameters:
12177 * @sock: Pointer to socket to be destroyed
12178 *
12179 * Return:
12180 * On error, may return EPROTONOSUPPORT, EINVAL.
12181 * EPROTONOSUPPORT if protocol specific destroy handler is not supported.
12182 * 0 otherwise
12183 */
12184 __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
12185 {
12186 struct sock *sk = (struct sock *)sock;
12187
12188 /* The locking semantics that allow for synchronous execution of the
12189 * destroy handlers are only supported for TCP and UDP.
12190 * Supporting protocols will need to acquire sock lock in the BPF context
12191 * prior to invoking this kfunc.
12192 */
12193 if (!sk->sk_prot->diag_destroy || (sk->sk_protocol != IPPROTO_TCP &&
12194 sk->sk_protocol != IPPROTO_UDP))
12195 return -EOPNOTSUPP;
12196
12197 return sk->sk_prot->diag_destroy(sk, ECONNABORTED);
12198 }
12199
12200 __bpf_kfunc_end_defs();
12201
12202 BTF_KFUNCS_START(bpf_sk_iter_kfunc_ids)
12203 BTF_ID_FLAGS(func, bpf_sock_destroy, KF_TRUSTED_ARGS)
12204 BTF_KFUNCS_END(bpf_sk_iter_kfunc_ids)
12205
12206 static int tracing_iter_filter(const struct bpf_prog *prog, u32 kfunc_id)
12207 {
12208 if (btf_id_set8_contains(&bpf_sk_iter_kfunc_ids, kfunc_id) &&
12209 prog->expected_attach_type != BPF_TRACE_ITER)
12210 return -EACCES;
12211 return 0;
12212 }
12213
12214 static const struct btf_kfunc_id_set bpf_sk_iter_kfunc_set = {
12215 .owner = THIS_MODULE,
12216 .set = &bpf_sk_iter_kfunc_ids,
12217 .filter = tracing_iter_filter,
12218 };
12219
12220 static int init_subsystem(void)
12221 {
12222 return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_sk_iter_kfunc_set);
12223 }
12224 late_initcall(init_subsystem);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
2025-01-09 23:55 ` kernel test robot
@ 2025-01-10 1:08 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2025-01-10 1:08 UTC (permalink / raw)
To: kernel test robot
Cc: bpf, linux-kernel, linux-security-module, oe-kbuild-all,
kernel-team, andrii, ast, daniel, martin.lau, kpsingh,
mattbobrowski, paul, jmorris, serge, memxor
On Thu, Jan 9, 2025 at 3:56 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Song,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on bpf-next/master]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Song-Liu/fs-xattr-bpf-Introduce-security-bpf-xattr-name-prefix/20250109-065503
> base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> patch link: https://lore.kernel.org/r/20250108225140.3467654-6-song%40kernel.org
> patch subject: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
> config: i386-buildonly-randconfig-005-20250110 (https://download.01.org/0day-ci/archive/20250110/202501100757.HDb5slrv-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250110/202501100757.HDb5slrv-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202501100757.HDb5slrv-lkp@intel.com/
>
> All error/warnings (new ones prefixed by >>):
>
> >> net/core/filter.c:12071:1: error: return type defaults to 'int' [-Werror=implicit-int]
> 12071 | BTF_HIDDEN_KFUNCS_START(bpf_kfunc_check_hidden_set_skb)
> | ^~~~~~~~~~~~~~~~~~~~~~~
Good catch.. Fixing this in v9.
Thanks,
Song
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
2025-01-08 22:51 ` [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb Song Liu
2025-01-09 23:55 ` kernel test robot
@ 2025-01-10 1:11 ` kernel test robot
2025-01-14 22:37 ` Andrii Nakryiko
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2025-01-10 1:11 UTC (permalink / raw)
To: Song Liu, bpf, linux-kernel, linux-security-module
Cc: oe-kbuild-all, kernel-team, andrii, ast, daniel, martin.lau,
kpsingh, mattbobrowski, paul, jmorris, serge, memxor, Song Liu
Hi Song,
kernel test robot noticed the following build errors:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Song-Liu/fs-xattr-bpf-Introduce-security-bpf-xattr-name-prefix/20250109-065503
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20250108225140.3467654-6-song%40kernel.org
patch subject: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
config: s390-randconfig-002-20250110 (https://download.01.org/0day-ci/archive/20250110/202501100813.5dE7y99c-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250110/202501100813.5dE7y99c-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501100813.5dE7y99c-lkp@intel.com/
All errors (new ones prefixed by >>):
net/core/filter.c:12071:1: error: return type defaults to 'int' [-Wimplicit-int]
12071 | BTF_HIDDEN_KFUNCS_START(bpf_kfunc_check_hidden_set_skb)
| ^~~~~~~~~~~~~~~~~~~~~~~
net/core/filter.c:12071:1: error: function declaration isn't a prototype [-Werror=strict-prototypes]
In file included from include/linux/btf.h:10,
from include/linux/bpf.h:28,
from include/linux/bpf_verifier.h:7,
from net/core/filter.c:21:
net/core/filter.c: In function 'BTF_HIDDEN_KFUNCS_START':
net/core/filter.c:12075:18: error: storage class specified for parameter 'bpf_kfunc_check_set_xdp'
12075 | BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
include/linux/btf_ids.h:235:46: error: parameter 'bpf_kfunc_check_set_xdp' is initialized
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~~~~~~~~
net/core/filter.c:12075:1: note: in expansion of macro 'BTF_KFUNCS_START'
12075 | BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12079:18: error: storage class specified for parameter 'bpf_kfunc_check_set_sock_addr'
12079 | BTF_KFUNCS_START(bpf_kfunc_check_set_sock_addr)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
include/linux/btf_ids.h:235:46: error: parameter 'bpf_kfunc_check_set_sock_addr' is initialized
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~~~~~~~~
net/core/filter.c:12079:1: note: in expansion of macro 'BTF_KFUNCS_START'
12079 | BTF_KFUNCS_START(bpf_kfunc_check_set_sock_addr)
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12083:18: error: storage class specified for parameter 'bpf_kfunc_check_set_tcp_reqsk'
12083 | BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
include/linux/btf_ids.h:235:46: error: parameter 'bpf_kfunc_check_set_tcp_reqsk' is initialized
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~~~~~~~~
net/core/filter.c:12083:1: note: in expansion of macro 'BTF_KFUNCS_START'
12083 | BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12087:13: error: storage class specified for parameter 'bpf_dynptr_from_skb_list'
12087 | BTF_ID_LIST(bpf_dynptr_from_skb_list)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:223:53: note: in definition of macro 'BTF_ID_LIST'
223 | #define BTF_ID_LIST(name) static u32 __maybe_unused name[64];
| ^~~~
net/core/filter.c:12092:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
12092 | {
| ^
net/core/filter.c:12122:38: error: storage class specified for parameter 'bpf_kfunc_set_skb'
12122 | static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12122:21: error: parameter 'bpf_kfunc_set_skb' is initialized
12122 | static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12125:24: error: 'bpf_kfunc_check_hidden_set_skb' undeclared (first use in this function); did you mean 'bpf_kfunc_check_set_skb'?
12125 | .hidden_set = &bpf_kfunc_check_hidden_set_skb,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| bpf_kfunc_check_set_skb
net/core/filter.c:12125:24: note: each undeclared identifier is reported only once for each function it appears in
net/core/filter.c:12126:19: error: 'bpf_kfunc_set_skb_remap' undeclared (first use in this function); did you mean 'bpf_kfunc_set_skb'?
12126 | .remap = &bpf_kfunc_set_skb_remap,
| ^~~~~~~~~~~~~~~~~~~~~~~
| bpf_kfunc_set_skb
net/core/filter.c:12129:38: error: storage class specified for parameter 'bpf_kfunc_set_xdp'
12129 | static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12129:21: error: parameter 'bpf_kfunc_set_xdp' is initialized
12129 | static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12134:38: error: storage class specified for parameter 'bpf_kfunc_set_sock_addr'
12134 | static const struct btf_kfunc_id_set bpf_kfunc_set_sock_addr = {
| ^~~~~~~~~~~~~~~~~~~~~~~
net/core/filter.c:12134:21: error: parameter 'bpf_kfunc_set_sock_addr' is initialized
12134 | static const struct btf_kfunc_id_set bpf_kfunc_set_sock_addr = {
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12139:38: error: storage class specified for parameter 'bpf_kfunc_set_tcp_reqsk'
12139 | static const struct btf_kfunc_id_set bpf_kfunc_set_tcp_reqsk = {
| ^~~~~~~~~~~~~~~~~~~~~~~
net/core/filter.c:12139:21: error: parameter 'bpf_kfunc_set_tcp_reqsk' is initialized
12139 | static const struct btf_kfunc_id_set bpf_kfunc_set_tcp_reqsk = {
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12145:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
12145 | {
| ^
In file included from include/linux/printk.h:6,
from include/asm-generic/bug.h:22,
from arch/s390/include/asm/bug.h:69,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from arch/s390/include/asm/cmpxchg.h:11,
from arch/s390/include/asm/atomic.h:16,
from include/linux/atomic.h:7,
from net/core/filter.c:20:
>> include/linux/init.h:218:17: error: storage class specified for parameter '__initcall__kmod_filter__1641_12164_bpf_kfunc_init7'
218 | __PASTE(__, \
| ^~
include/linux/init.h:269:27: note: in definition of macro '____define_initcall'
269 | static initcall_t __name __used \
| ^~~~~~
include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^~~~~~~~
include/linux/init.h:218:9: note: in expansion of macro '__PASTE'
218 | __PASTE(__, \
| ^~~~~~~
include/linux/init.h:276:17: note: in expansion of macro '__initcall_name'
276 | __initcall_name(initcall, __iid, id), \
| ^~~~~~~~~~~~~~~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12164:1: note: in expansion of macro 'late_initcall'
12164 | late_initcall(bpf_kfunc_init);
| ^~~~~~~~~~~~~
>> net/core/filter.c:12164:1: error: parameter '__initcall__kmod_filter__1641_12164_bpf_kfunc_init7' is initialized
net/core/filter.c:12164:1: warning: 'used' attribute ignored [-Wattributes]
>> include/linux/init.h:218:17: error: section attribute not allowed for '__initcall__kmod_filter__1641_12164_bpf_kfunc_init7'
218 | __PASTE(__, \
| ^~
include/linux/init.h:269:27: note: in definition of macro '____define_initcall'
269 | static initcall_t __name __used \
| ^~~~~~
include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^~~~~~~~
include/linux/init.h:218:9: note: in expansion of macro '__PASTE'
218 | __PASTE(__, \
| ^~~~~~~
include/linux/init.h:276:17: note: in expansion of macro '__initcall_name'
276 | __initcall_name(initcall, __iid, id), \
| ^~~~~~~~~~~~~~~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12164:1: note: in expansion of macro 'late_initcall'
12164 | late_initcall(bpf_kfunc_init);
| ^~~~~~~~~~~~~
net/core/filter.c:12164:15: error: 'bpf_kfunc_init' undeclared (first use in this function); did you mean 'bpf_func_info'?
12164 | late_initcall(bpf_kfunc_init);
| ^~~~~~~~~~~~~~
include/linux/init.h:270:55: note: in definition of macro '____define_initcall'
270 | __attribute__((__section__(__sec))) = fn;
| ^~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12164:1: note: in expansion of macro 'late_initcall'
12164 | late_initcall(bpf_kfunc_init);
| ^~~~~~~~~~~~~
net/core/filter.c:12164:30: error: expected declaration specifiers before ';' token
12164 | late_initcall(bpf_kfunc_init);
| ^
In file included from include/linux/compiler_types.h:174,
from <command-line>:
include/linux/compiler-gcc.h:134:33: error: expected declaration specifiers before '#pragma'
134 | #define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
| ^~~~~~~
include/linux/compiler_types.h:557:25: note: in expansion of macro '__diag'
557 | #define __diag_push() __diag(push)
| ^~~~~~
include/linux/btf.h:89:9: note: in expansion of macro '__diag_push'
89 | __diag_push(); \
| ^~~~~~~~~~~
net/core/filter.c:12166:1: note: in expansion of macro '__bpf_kfunc_start_defs'
12166 | __bpf_kfunc_start_defs();
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler-gcc.h:134:33: error: expected declaration specifiers before '#pragma'
134 | #define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
| ^~~~~~~
include/linux/compiler-gcc.h:143:9: note: in expansion of macro '__diag'
143 | __diag(__diag_GCC_ignore option)
| ^~~~~~
include/linux/btf.h:90:9: note: in expansion of macro '__diag_ignore_all'
90 | __diag_ignore_all("-Wmissing-declarations", \
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12166:1: note: in expansion of macro '__bpf_kfunc_start_defs'
12166 | __bpf_kfunc_start_defs();
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler-gcc.h:134:33: error: expected declaration specifiers before '#pragma'
134 | #define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
| ^~~~~~~
include/linux/compiler-gcc.h:143:9: note: in expansion of macro '__diag'
143 | __diag(__diag_GCC_ignore option)
| ^~~~~~
include/linux/btf.h:92:9: note: in expansion of macro '__diag_ignore_all'
92 | __diag_ignore_all("-Wmissing-prototypes", \
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12166:1: note: in expansion of macro '__bpf_kfunc_start_defs'
12166 | __bpf_kfunc_start_defs();
| ^~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/compiler_types.h:89:
include/linux/compiler_attributes.h:349:41: error: expected declaration specifiers before '__attribute__'
349 | #define __used __attribute__((__used__))
| ^~~~~~~~~~~~~
include/linux/btf.h:86:21: note: in expansion of macro '__used'
86 | #define __bpf_kfunc __used __retain noinline
| ^~~~~~
net/core/filter.c:12184:1: note: in expansion of macro '__bpf_kfunc'
12184 | __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
| ^~~~~~~~~~~
include/linux/compiler-gcc.h:134:33: error: expected declaration specifiers before '#pragma'
134 | #define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
| ^~~~~~~
include/linux/compiler_types.h:558:25: note: in expansion of macro '__diag'
558 | #define __diag_pop() __diag(pop)
| ^~~~~~
include/linux/btf.h:95:32: note: in expansion of macro '__diag_pop'
95 | #define __bpf_kfunc_end_defs() __diag_pop()
| ^~~~~~~~~~
net/core/filter.c:12200:1: note: in expansion of macro '__bpf_kfunc_end_defs'
12200 | __bpf_kfunc_end_defs();
| ^~~~~~~~~~~~~~~~~~~~
net/core/filter.c:12202:18: error: storage class specified for parameter 'bpf_sk_iter_kfunc_ids'
12202 | BTF_KFUNCS_START(bpf_sk_iter_kfunc_ids)
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
include/linux/btf_ids.h:235:46: error: parameter 'bpf_sk_iter_kfunc_ids' is initialized
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~~~~~~~~
net/core/filter.c:12202:1: note: in expansion of macro 'BTF_KFUNCS_START'
12202 | BTF_KFUNCS_START(bpf_sk_iter_kfunc_ids)
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12207:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
12207 | {
| ^
net/core/filter.c:12214:38: error: storage class specified for parameter 'bpf_sk_iter_kfunc_set'
12214 | static const struct btf_kfunc_id_set bpf_sk_iter_kfunc_set = {
| ^~~~~~~~~~~~~~~~~~~~~
net/core/filter.c:12214:21: error: parameter 'bpf_sk_iter_kfunc_set' is initialized
12214 | static const struct btf_kfunc_id_set bpf_sk_iter_kfunc_set = {
| ^~~~~~~~~~~~~~~~
net/core/filter.c:12217:19: error: 'tracing_iter_filter' undeclared (first use in this function)
12217 | .filter = tracing_iter_filter,
| ^~~~~~~~~~~~~~~~~~~
net/core/filter.c:12221:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
12221 | {
| ^
>> include/linux/init.h:218:17: error: storage class specified for parameter '__initcall__kmod_filter__1642_12224_init_subsystem7'
218 | __PASTE(__, \
| ^~
include/linux/init.h:269:27: note: in definition of macro '____define_initcall'
269 | static initcall_t __name __used \
| ^~~~~~
include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^~~~~~~~
include/linux/init.h:218:9: note: in expansion of macro '__PASTE'
218 | __PASTE(__, \
| ^~~~~~~
include/linux/init.h:276:17: note: in expansion of macro '__initcall_name'
276 | __initcall_name(initcall, __iid, id), \
| ^~~~~~~~~~~~~~~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12224:1: note: in expansion of macro 'late_initcall'
12224 | late_initcall(init_subsystem);
| ^~~~~~~~~~~~~
>> net/core/filter.c:12224:1: error: parameter '__initcall__kmod_filter__1642_12224_init_subsystem7' is initialized
net/core/filter.c:12224:1: warning: 'used' attribute ignored [-Wattributes]
>> include/linux/init.h:218:17: error: section attribute not allowed for '__initcall__kmod_filter__1642_12224_init_subsystem7'
218 | __PASTE(__, \
| ^~
include/linux/init.h:269:27: note: in definition of macro '____define_initcall'
269 | static initcall_t __name __used \
| ^~~~~~
include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^~~~~~~~
include/linux/init.h:218:9: note: in expansion of macro '__PASTE'
218 | __PASTE(__, \
| ^~~~~~~
include/linux/init.h:276:17: note: in expansion of macro '__initcall_name'
276 | __initcall_name(initcall, __iid, id), \
| ^~~~~~~~~~~~~~~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12224:1: note: in expansion of macro 'late_initcall'
12224 | late_initcall(init_subsystem);
| ^~~~~~~~~~~~~
net/core/filter.c:12224:15: error: 'init_subsystem' undeclared (first use in this function)
12224 | late_initcall(init_subsystem);
| ^~~~~~~~~~~~~~
include/linux/init.h:270:55: note: in definition of macro '____define_initcall'
270 | __attribute__((__section__(__sec))) = fn;
| ^~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12224:1: note: in expansion of macro 'late_initcall'
12224 | late_initcall(init_subsystem);
| ^~~~~~~~~~~~~
net/core/filter.c:12224:30: error: expected declaration specifiers before ';' token
12224 | late_initcall(init_subsystem);
| ^
>> include/linux/init.h:218:17: error: declaration for parameter '__initcall__kmod_filter__1642_12224_init_subsystem7' but no such parameter
218 | __PASTE(__, \
| ^~
include/linux/init.h:269:27: note: in definition of macro '____define_initcall'
269 | static initcall_t __name __used \
| ^~~~~~
include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^~~~~~~~
include/linux/init.h:218:9: note: in expansion of macro '__PASTE'
218 | __PASTE(__, \
| ^~~~~~~
include/linux/init.h:276:17: note: in expansion of macro '__initcall_name'
276 | __initcall_name(initcall, __iid, id), \
| ^~~~~~~~~~~~~~~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12224:1: note: in expansion of macro 'late_initcall'
12224 | late_initcall(init_subsystem);
| ^~~~~~~~~~~~~
net/core/filter.c:12214:38: error: declaration for parameter 'bpf_sk_iter_kfunc_set' but no such parameter
12214 | static const struct btf_kfunc_id_set bpf_sk_iter_kfunc_set = {
| ^~~~~~~~~~~~~~~~~~~~~
net/core/filter.c:12202:18: error: declaration for parameter 'bpf_sk_iter_kfunc_ids' but no such parameter
12202 | BTF_KFUNCS_START(bpf_sk_iter_kfunc_ids)
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
>> include/linux/init.h:218:17: error: declaration for parameter '__initcall__kmod_filter__1641_12164_bpf_kfunc_init7' but no such parameter
218 | __PASTE(__, \
| ^~
include/linux/init.h:269:27: note: in definition of macro '____define_initcall'
269 | static initcall_t __name __used \
| ^~~~~~
include/linux/compiler_types.h:84:22: note: in expansion of macro '___PASTE'
84 | #define __PASTE(a,b) ___PASTE(a,b)
| ^~~~~~~~
include/linux/init.h:218:9: note: in expansion of macro '__PASTE'
218 | __PASTE(__, \
| ^~~~~~~
include/linux/init.h:276:17: note: in expansion of macro '__initcall_name'
276 | __initcall_name(initcall, __iid, id), \
| ^~~~~~~~~~~~~~~
include/linux/init.h:280:9: note: in expansion of macro '__unique_initcall'
280 | __unique_initcall(fn, id, __sec, __initcall_id(fn))
| ^~~~~~~~~~~~~~~~~
include/linux/init.h:282:35: note: in expansion of macro '___define_initcall'
282 | #define __define_initcall(fn, id) ___define_initcall(fn, id, .initcall##id)
| ^~~~~~~~~~~~~~~~~~
include/linux/init.h:313:41: note: in expansion of macro '__define_initcall'
313 | #define late_initcall(fn) __define_initcall(fn, 7)
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12164:1: note: in expansion of macro 'late_initcall'
12164 | late_initcall(bpf_kfunc_init);
| ^~~~~~~~~~~~~
net/core/filter.c:12139:38: error: declaration for parameter 'bpf_kfunc_set_tcp_reqsk' but no such parameter
12139 | static const struct btf_kfunc_id_set bpf_kfunc_set_tcp_reqsk = {
| ^~~~~~~~~~~~~~~~~~~~~~~
net/core/filter.c:12134:38: error: declaration for parameter 'bpf_kfunc_set_sock_addr' but no such parameter
12134 | static const struct btf_kfunc_id_set bpf_kfunc_set_sock_addr = {
| ^~~~~~~~~~~~~~~~~~~~~~~
net/core/filter.c:12129:38: error: declaration for parameter 'bpf_kfunc_set_xdp' but no such parameter
12129 | static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12122:38: error: declaration for parameter 'bpf_kfunc_set_skb' but no such parameter
12122 | static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
| ^~~~~~~~~~~~~~~~~
net/core/filter.c:12087:13: error: declaration for parameter 'bpf_dynptr_from_skb_list' but no such parameter
12087 | BTF_ID_LIST(bpf_dynptr_from_skb_list)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:223:53: note: in definition of macro 'BTF_ID_LIST'
223 | #define BTF_ID_LIST(name) static u32 __maybe_unused name[64];
| ^~~~
net/core/filter.c:12083:18: error: declaration for parameter 'bpf_kfunc_check_set_tcp_reqsk' but no such parameter
12083 | BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
net/core/filter.c:12079:18: error: declaration for parameter 'bpf_kfunc_check_set_sock_addr' but no such parameter
12079 | BTF_KFUNCS_START(bpf_kfunc_check_set_sock_addr)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
net/core/filter.c:12075:18: error: declaration for parameter 'bpf_kfunc_check_set_xdp' but no such parameter
12075 | BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/btf_ids.h:235:73: note: in definition of macro 'BTF_KFUNCS_START'
235 | #define BTF_KFUNCS_START(name) static struct btf_id_set8 __maybe_unused name = { .flags = BTF_SET8_KFUNCS };
| ^~~~
net/core/filter.c:12225: error: expected '{' at end of input
net/core/filter.c:12225: warning: control reaches end of non-void function [-Wreturn-type]
cc1: some warnings being treated as errors
vim +/__initcall__kmod_filter__1641_12164_bpf_kfunc_init7 +12164 net/core/filter.c
e472f88891abbc Kuniyuki Iwashima 2024-01-15 12143
b5964b968ac64c Joanne Koong 2023-03-01 12144 static int __init bpf_kfunc_init(void)
b5964b968ac64c Joanne Koong 2023-03-01 12145 {
b5964b968ac64c Joanne Koong 2023-03-01 12146 int ret;
b5964b968ac64c Joanne Koong 2023-03-01 12147
b5964b968ac64c Joanne Koong 2023-03-01 12148 ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_kfunc_set_skb);
b5964b968ac64c Joanne Koong 2023-03-01 12149 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_ACT, &bpf_kfunc_set_skb);
b5964b968ac64c Joanne Koong 2023-03-01 12150 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SK_SKB, &bpf_kfunc_set_skb);
b5964b968ac64c Joanne Koong 2023-03-01 12151 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SOCKET_FILTER, &bpf_kfunc_set_skb);
b5964b968ac64c Joanne Koong 2023-03-01 12152 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SKB, &bpf_kfunc_set_skb);
b5964b968ac64c Joanne Koong 2023-03-01 12153 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_OUT, &bpf_kfunc_set_skb);
b5964b968ac64c Joanne Koong 2023-03-01 12154 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_IN, &bpf_kfunc_set_skb);
b5964b968ac64c Joanne Koong 2023-03-01 12155 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_XMIT, &bpf_kfunc_set_skb);
05421aecd4ed65 Joanne Koong 2023-03-01 12156 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_SEG6LOCAL, &bpf_kfunc_set_skb);
fd9c663b9ad67d Florian Westphal 2023-04-21 12157 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_NETFILTER, &bpf_kfunc_set_skb);
ffc83860d8c097 Philo Lu 2024-09-11 12158 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_kfunc_set_skb);
53e380d2144190 Daan De Meyer 2023-10-11 12159 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &bpf_kfunc_set_xdp);
e472f88891abbc Kuniyuki Iwashima 2024-01-15 12160 ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
53e380d2144190 Daan De Meyer 2023-10-11 12161 &bpf_kfunc_set_sock_addr);
e472f88891abbc Kuniyuki Iwashima 2024-01-15 12162 return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_kfunc_set_tcp_reqsk);
b5964b968ac64c Joanne Koong 2023-03-01 12163 }
b5964b968ac64c Joanne Koong 2023-03-01 @12164 late_initcall(bpf_kfunc_init);
4ddbcb886268af Aditi Ghag 2023-05-19 12165
391145ba2accc4 Dave Marchevsky 2023-10-31 12166 __bpf_kfunc_start_defs();
4ddbcb886268af Aditi Ghag 2023-05-19 12167
4ddbcb886268af Aditi Ghag 2023-05-19 12168 /* bpf_sock_destroy: Destroy the given socket with ECONNABORTED error code.
4ddbcb886268af Aditi Ghag 2023-05-19 12169 *
4ddbcb886268af Aditi Ghag 2023-05-19 12170 * The function expects a non-NULL pointer to a socket, and invokes the
4ddbcb886268af Aditi Ghag 2023-05-19 12171 * protocol specific socket destroy handlers.
4ddbcb886268af Aditi Ghag 2023-05-19 12172 *
4ddbcb886268af Aditi Ghag 2023-05-19 12173 * The helper can only be called from BPF contexts that have acquired the socket
4ddbcb886268af Aditi Ghag 2023-05-19 12174 * locks.
4ddbcb886268af Aditi Ghag 2023-05-19 12175 *
4ddbcb886268af Aditi Ghag 2023-05-19 12176 * Parameters:
4ddbcb886268af Aditi Ghag 2023-05-19 12177 * @sock: Pointer to socket to be destroyed
4ddbcb886268af Aditi Ghag 2023-05-19 12178 *
4ddbcb886268af Aditi Ghag 2023-05-19 12179 * Return:
4ddbcb886268af Aditi Ghag 2023-05-19 12180 * On error, may return EPROTONOSUPPORT, EINVAL.
4ddbcb886268af Aditi Ghag 2023-05-19 12181 * EPROTONOSUPPORT if protocol specific destroy handler is not supported.
4ddbcb886268af Aditi Ghag 2023-05-19 12182 * 0 otherwise
4ddbcb886268af Aditi Ghag 2023-05-19 12183 */
4ddbcb886268af Aditi Ghag 2023-05-19 12184 __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
4ddbcb886268af Aditi Ghag 2023-05-19 12185 {
4ddbcb886268af Aditi Ghag 2023-05-19 12186 struct sock *sk = (struct sock *)sock;
4ddbcb886268af Aditi Ghag 2023-05-19 12187
4ddbcb886268af Aditi Ghag 2023-05-19 12188 /* The locking semantics that allow for synchronous execution of the
4ddbcb886268af Aditi Ghag 2023-05-19 12189 * destroy handlers are only supported for TCP and UDP.
4ddbcb886268af Aditi Ghag 2023-05-19 12190 * Supporting protocols will need to acquire sock lock in the BPF context
4ddbcb886268af Aditi Ghag 2023-05-19 12191 * prior to invoking this kfunc.
4ddbcb886268af Aditi Ghag 2023-05-19 12192 */
4ddbcb886268af Aditi Ghag 2023-05-19 12193 if (!sk->sk_prot->diag_destroy || (sk->sk_protocol != IPPROTO_TCP &&
4ddbcb886268af Aditi Ghag 2023-05-19 12194 sk->sk_protocol != IPPROTO_UDP))
4ddbcb886268af Aditi Ghag 2023-05-19 12195 return -EOPNOTSUPP;
4ddbcb886268af Aditi Ghag 2023-05-19 12196
4ddbcb886268af Aditi Ghag 2023-05-19 12197 return sk->sk_prot->diag_destroy(sk, ECONNABORTED);
4ddbcb886268af Aditi Ghag 2023-05-19 12198 }
4ddbcb886268af Aditi Ghag 2023-05-19 12199
391145ba2accc4 Dave Marchevsky 2023-10-31 12200 __bpf_kfunc_end_defs();
4ddbcb886268af Aditi Ghag 2023-05-19 12201
6f3189f38a3e99 Daniel Xu 2024-01-28 12202 BTF_KFUNCS_START(bpf_sk_iter_kfunc_ids)
4ddbcb886268af Aditi Ghag 2023-05-19 12203 BTF_ID_FLAGS(func, bpf_sock_destroy, KF_TRUSTED_ARGS)
6f3189f38a3e99 Daniel Xu 2024-01-28 12204 BTF_KFUNCS_END(bpf_sk_iter_kfunc_ids)
4ddbcb886268af Aditi Ghag 2023-05-19 12205
4ddbcb886268af Aditi Ghag 2023-05-19 12206 static int tracing_iter_filter(const struct bpf_prog *prog, u32 kfunc_id)
4ddbcb886268af Aditi Ghag 2023-05-19 12207 {
4ddbcb886268af Aditi Ghag 2023-05-19 12208 if (btf_id_set8_contains(&bpf_sk_iter_kfunc_ids, kfunc_id) &&
4ddbcb886268af Aditi Ghag 2023-05-19 12209 prog->expected_attach_type != BPF_TRACE_ITER)
4ddbcb886268af Aditi Ghag 2023-05-19 12210 return -EACCES;
4ddbcb886268af Aditi Ghag 2023-05-19 12211 return 0;
4ddbcb886268af Aditi Ghag 2023-05-19 12212 }
4ddbcb886268af Aditi Ghag 2023-05-19 12213
4ddbcb886268af Aditi Ghag 2023-05-19 12214 static const struct btf_kfunc_id_set bpf_sk_iter_kfunc_set = {
4ddbcb886268af Aditi Ghag 2023-05-19 12215 .owner = THIS_MODULE,
4ddbcb886268af Aditi Ghag 2023-05-19 12216 .set = &bpf_sk_iter_kfunc_ids,
4ddbcb886268af Aditi Ghag 2023-05-19 12217 .filter = tracing_iter_filter,
4ddbcb886268af Aditi Ghag 2023-05-19 12218 };
4ddbcb886268af Aditi Ghag 2023-05-19 12219
4ddbcb886268af Aditi Ghag 2023-05-19 12220 static int init_subsystem(void)
4ddbcb886268af Aditi Ghag 2023-05-19 12221 {
4ddbcb886268af Aditi Ghag 2023-05-19 12222 return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_sk_iter_kfunc_set);
4ddbcb886268af Aditi Ghag 2023-05-19 12223 }
4ddbcb886268af Aditi Ghag 2023-05-19 @12224 late_initcall(init_subsystem);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
2025-01-08 22:51 ` [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb Song Liu
2025-01-09 23:55 ` kernel test robot
2025-01-10 1:11 ` kernel test robot
@ 2025-01-14 22:37 ` Andrii Nakryiko
2025-01-14 23:03 ` Song Liu
2 siblings, 1 reply; 14+ messages in thread
From: Andrii Nakryiko @ 2025-01-14 22:37 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-kernel, linux-security-module, kernel-team, andrii,
ast, daniel, martin.lau, kpsingh, mattbobrowski, paul, jmorris,
serge, memxor
On Wed, Jan 8, 2025 at 2:52 PM Song Liu <song@kernel.org> wrote:
>
> btf_kfunc_id_set.remap can pick proper version of a kfunc for the calling
> context. Use this logic to select bpf_dynptr_from_skb or
> bpf_dynptr_from_skb_rdonly. This will make the verifier simpler.
>
> Unfortunately, btf_kfunc_id_set.remap cannot cover the DYNPTR_TYPE_SKB
> logic in check_kfunc_args(). This can be addressed later.
>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
> kernel/bpf/verifier.c | 25 ++++++----------------
> net/core/filter.c | 49 +++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 51 insertions(+), 23 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index c321fd25fca3..95b0847191fe 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11677,6 +11677,7 @@ enum special_kfunc_type {
> KF_bpf_rbtree_add_impl,
> KF_bpf_rbtree_first,
> KF_bpf_dynptr_from_skb,
> + KF_bpf_dynptr_from_skb_rdonly,
> KF_bpf_dynptr_from_xdp,
> KF_bpf_dynptr_slice,
> KF_bpf_dynptr_slice_rdwr,
> @@ -11712,6 +11713,7 @@ BTF_ID(func, bpf_rbtree_add_impl)
> BTF_ID(func, bpf_rbtree_first)
> #ifdef CONFIG_NET
> BTF_ID(func, bpf_dynptr_from_skb)
> +BTF_ID(func, bpf_dynptr_from_skb_rdonly)
> BTF_ID(func, bpf_dynptr_from_xdp)
> #endif
> BTF_ID(func, bpf_dynptr_slice)
> @@ -11743,10 +11745,12 @@ BTF_ID(func, bpf_rbtree_add_impl)
> BTF_ID(func, bpf_rbtree_first)
> #ifdef CONFIG_NET
> BTF_ID(func, bpf_dynptr_from_skb)
> +BTF_ID(func, bpf_dynptr_from_skb_rdonly)
> BTF_ID(func, bpf_dynptr_from_xdp)
> #else
> BTF_ID_UNUSED
> BTF_ID_UNUSED
> +BTF_ID_UNUSED
> #endif
> BTF_ID(func, bpf_dynptr_slice)
> BTF_ID(func, bpf_dynptr_slice_rdwr)
> @@ -12668,7 +12672,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> if (is_kfunc_arg_uninit(btf, &args[i]))
> dynptr_arg_type |= MEM_UNINIT;
>
> - if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) {
> + if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb] ||
> + meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb_rdonly]) {
> dynptr_arg_type |= DYNPTR_TYPE_SKB;
> } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_xdp]) {
> dynptr_arg_type |= DYNPTR_TYPE_XDP;
> @@ -20821,9 +20826,7 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
> u32 func_id, u16 offset, unsigned long *addr)
> {
> struct bpf_prog *prog = env->prog;
> - bool seen_direct_write;
> void *xdp_kfunc;
> - bool is_rdonly;
>
> if (bpf_dev_bound_kfunc_id(func_id)) {
> xdp_kfunc = bpf_dev_bound_resolve_kfunc(prog, func_id);
> @@ -20833,22 +20836,6 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
> }
> /* fallback to default kfunc when not supported by netdev */
> }
> -
> - if (offset)
> - return;
> -
> - if (func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) {
> - seen_direct_write = env->seen_direct_write;
> - is_rdonly = !may_access_direct_pkt_data(env, NULL, BPF_WRITE);
> -
> - if (is_rdonly)
> - *addr = (unsigned long)bpf_dynptr_from_skb_rdonly;
> -
> - /* restore env->seen_direct_write to its original value, since
> - * may_access_direct_pkt_data mutates it
> - */
> - env->seen_direct_write = seen_direct_write;
is it safe to remove this special seen_direct_write part of logic?
> - }
> }
>
> static void __fixup_collection_insert_kfunc(struct bpf_insn_aux_data *insn_aux,
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 21131ec25f24..f12bcc1b21d1 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -12047,10 +12047,8 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk,
> #endif
> }
>
> -__bpf_kfunc_end_defs();
> -
> -int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
> - struct bpf_dynptr *ptr__uninit)
> +__bpf_kfunc int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
> + struct bpf_dynptr *ptr__uninit)
> {
> struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit;
> int err;
> @@ -12064,10 +12062,16 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
> return 0;
> }
>
> +__bpf_kfunc_end_defs();
> +
> BTF_KFUNCS_START(bpf_kfunc_check_set_skb)
> BTF_ID_FLAGS(func, bpf_dynptr_from_skb, KF_TRUSTED_ARGS)
> BTF_KFUNCS_END(bpf_kfunc_check_set_skb)
>
> +BTF_HIDDEN_KFUNCS_START(bpf_kfunc_check_hidden_set_skb)
> +BTF_ID_FLAGS(func, bpf_dynptr_from_skb_rdonly, KF_TRUSTED_ARGS)
> +BTF_KFUNCS_END(bpf_kfunc_check_hidden_set_skb)
> +
> BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
> BTF_ID_FLAGS(func, bpf_dynptr_from_xdp)
> BTF_KFUNCS_END(bpf_kfunc_check_set_xdp)
> @@ -12080,9 +12084,46 @@ BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
> BTF_ID_FLAGS(func, bpf_sk_assign_tcp_reqsk, KF_TRUSTED_ARGS)
> BTF_KFUNCS_END(bpf_kfunc_check_set_tcp_reqsk)
>
> +BTF_ID_LIST(bpf_dynptr_from_skb_list)
> +BTF_ID(func, bpf_dynptr_from_skb)
> +BTF_ID(func, bpf_dynptr_from_skb_rdonly)
> +
> +static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id)
> +{
> + if (kfunc_id != bpf_dynptr_from_skb_list[0])
> + return 0;
> +
> + switch (resolve_prog_type(prog)) {
> + /* Program types only with direct read access go here! */
> + case BPF_PROG_TYPE_LWT_IN:
> + case BPF_PROG_TYPE_LWT_OUT:
> + case BPF_PROG_TYPE_LWT_SEG6LOCAL:
> + case BPF_PROG_TYPE_SK_REUSEPORT:
> + case BPF_PROG_TYPE_FLOW_DISSECTOR:
> + case BPF_PROG_TYPE_CGROUP_SKB:
> + return bpf_dynptr_from_skb_list[1];
> +
> + /* Program types with direct read + write access go here! */
> + case BPF_PROG_TYPE_SCHED_CLS:
> + case BPF_PROG_TYPE_SCHED_ACT:
> + case BPF_PROG_TYPE_XDP:
> + case BPF_PROG_TYPE_LWT_XMIT:
> + case BPF_PROG_TYPE_SK_SKB:
> + case BPF_PROG_TYPE_SK_MSG:
> + case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> + return kfunc_id;
> +
> + default:
> + break;
> + }
> + return bpf_dynptr_from_skb_list[1];
> +}
I'd personally prefer the approach we have with BPF helpers, where
each program type has a function that handles all helpers (identified
by its ID), and then we can use C code sharing to minimize duplication
of code.
With this approach it seems like we'll have more duplication and we'll
need to repeat these program type-based large switches for various
small sets of kfuncs, no?
> +
> static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
> .owner = THIS_MODULE,
> .set = &bpf_kfunc_check_set_skb,
> + .hidden_set = &bpf_kfunc_check_hidden_set_skb,
> + .remap = &bpf_kfunc_set_skb_remap,
> };
>
> static const struct btf_kfunc_id_set bpf_kfunc_set_xdp = {
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
2025-01-14 22:37 ` Andrii Nakryiko
@ 2025-01-14 23:03 ` Song Liu
2025-01-14 23:40 ` Andrii Nakryiko
0 siblings, 1 reply; 14+ messages in thread
From: Song Liu @ 2025-01-14 23:03 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Song Liu, bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, Kernel Team,
andrii@kernel.org, ast@kernel.org, daniel@iogearbox.net,
martin.lau@linux.dev, kpsingh@kernel.org,
mattbobrowski@google.com, paul@paul-moore.com, jmorris@namei.org,
serge@hallyn.com, memxor@gmail.com
> On Jan 14, 2025, at 2:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
[...]
>>
>> if (bpf_dev_bound_kfunc_id(func_id)) {
>> xdp_kfunc = bpf_dev_bound_resolve_kfunc(prog, func_id);
>> @@ -20833,22 +20836,6 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
>> }
>> /* fallback to default kfunc when not supported by netdev */
>> }
>> -
>> - if (offset)
>> - return;
>> -
>> - if (func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) {
>> - seen_direct_write = env->seen_direct_write;
>> - is_rdonly = !may_access_direct_pkt_data(env, NULL, BPF_WRITE);
>> -
>> - if (is_rdonly)
>> - *addr = (unsigned long)bpf_dynptr_from_skb_rdonly;
>> -
>> - /* restore env->seen_direct_write to its original value, since
>> - * may_access_direct_pkt_data mutates it
>> - */
>> - env->seen_direct_write = seen_direct_write;
>
> is it safe to remove this special seen_direct_write part of logic?
We need to save and restore seen_direct_write because
may_access_direct_pkt_data() mutates it. If we do not call
may_access_direct_pkt_data() here, as after this patch, we don't need to
save and restore seen_direct_write.
>
>> - }
>> }
>>
>> static void __fixup_collection_insert_kfunc(struct bpf_insn_aux_data *insn_aux,
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 21131ec25f24..f12bcc1b21d1 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -12047,10 +12047,8 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk,
>> #endif
>> }
>>
>> -__bpf_kfunc_end_defs();
>> -
>> -int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
>> - struct bpf_dynptr *ptr__uninit)
>> +__bpf_kfunc int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
>> + struct bpf_dynptr *ptr__uninit)
>> {
>> struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit;
>> int err;
>> @@ -12064,10 +12062,16 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
>> return 0;
>> }
[...]
>> +
>> +static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id)
>> +{
>> + if (kfunc_id != bpf_dynptr_from_skb_list[0])
>> + return 0;
>> +
>> + switch (resolve_prog_type(prog)) {
>> + /* Program types only with direct read access go here! */
>> + case BPF_PROG_TYPE_LWT_IN:
>> + case BPF_PROG_TYPE_LWT_OUT:
>> + case BPF_PROG_TYPE_LWT_SEG6LOCAL:
>> + case BPF_PROG_TYPE_SK_REUSEPORT:
>> + case BPF_PROG_TYPE_FLOW_DISSECTOR:
>> + case BPF_PROG_TYPE_CGROUP_SKB:
>> + return bpf_dynptr_from_skb_list[1];
>> +
>> + /* Program types with direct read + write access go here! */
>> + case BPF_PROG_TYPE_SCHED_CLS:
>> + case BPF_PROG_TYPE_SCHED_ACT:
>> + case BPF_PROG_TYPE_XDP:
>> + case BPF_PROG_TYPE_LWT_XMIT:
>> + case BPF_PROG_TYPE_SK_SKB:
>> + case BPF_PROG_TYPE_SK_MSG:
>> + case BPF_PROG_TYPE_CGROUP_SOCKOPT:
>> + return kfunc_id;
>> +
>> + default:
>> + break;
>> + }
>> + return bpf_dynptr_from_skb_list[1];
>> +}
>
> I'd personally prefer the approach we have with BPF helpers, where
> each program type has a function that handles all helpers (identified
> by its ID), and then we can use C code sharing to minimize duplication
> of code.
Different hooks of the same program type, especially struct_ops, may
not have same access to different kfuncs. Therefore, I am not sure
whether the approach with helpers can scale in the long term. At the
moment, we use special_kfunc_[type|set|list] to handle special cases.
But I am afraid this approach cannot work well with more struct_ops
and kfuncs.
>
> With this approach it seems like we'll have more duplication and we'll
> need to repeat these program type-based large switches for various
> small sets of kfuncs, no?
The motivation is to make the verification of kfuncs more modular, so
that each set of kfuncs handle their verification as much as possible.
I think the code duplication here (bpf_kfunc_set_skb_remap) is not a
common problem. And we can actually reduce duplication with some
simple helpers.
Does this make sense?
Thanks,
Song
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb
2025-01-14 23:03 ` Song Liu
@ 2025-01-14 23:40 ` Andrii Nakryiko
0 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2025-01-14 23:40 UTC (permalink / raw)
To: Song Liu
Cc: Song Liu, bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, Kernel Team,
andrii@kernel.org, ast@kernel.org, daniel@iogearbox.net,
martin.lau@linux.dev, kpsingh@kernel.org,
mattbobrowski@google.com, paul@paul-moore.com, jmorris@namei.org,
serge@hallyn.com, memxor@gmail.com
On Tue, Jan 14, 2025 at 3:03 PM Song Liu <songliubraving@meta.com> wrote:
>
>
>
> > On Jan 14, 2025, at 2:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
>
> [...]
>
> >>
> >> if (bpf_dev_bound_kfunc_id(func_id)) {
> >> xdp_kfunc = bpf_dev_bound_resolve_kfunc(prog, func_id);
> >> @@ -20833,22 +20836,6 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
> >> }
> >> /* fallback to default kfunc when not supported by netdev */
> >> }
> >> -
> >> - if (offset)
> >> - return;
> >> -
> >> - if (func_id == special_kfunc_list[KF_bpf_dynptr_from_skb]) {
> >> - seen_direct_write = env->seen_direct_write;
> >> - is_rdonly = !may_access_direct_pkt_data(env, NULL, BPF_WRITE);
> >> -
> >> - if (is_rdonly)
> >> - *addr = (unsigned long)bpf_dynptr_from_skb_rdonly;
> >> -
> >> - /* restore env->seen_direct_write to its original value, since
> >> - * may_access_direct_pkt_data mutates it
> >> - */
> >> - env->seen_direct_write = seen_direct_write;
> >
> > is it safe to remove this special seen_direct_write part of logic?
>
> We need to save and restore seen_direct_write because
> may_access_direct_pkt_data() mutates it. If we do not call
> may_access_direct_pkt_data() here, as after this patch, we don't need to
> save and restore seen_direct_write.
>
ah, existing logic is quite convoluted (and that resolve_prog_type()
bit is another gotcha that's easy to miss), ok, so we used some
side-effecting function for simulating side effect-free check...
> >
> >> - }
> >> }
> >>
> >> static void __fixup_collection_insert_kfunc(struct bpf_insn_aux_data *insn_aux,
> >> diff --git a/net/core/filter.c b/net/core/filter.c
> >> index 21131ec25f24..f12bcc1b21d1 100644
> >> --- a/net/core/filter.c
> >> +++ b/net/core/filter.c
> >> @@ -12047,10 +12047,8 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk,
> >> #endif
> >> }
> >>
> >> -__bpf_kfunc_end_defs();
> >> -
> >> -int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
> >> - struct bpf_dynptr *ptr__uninit)
> >> +__bpf_kfunc int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
> >> + struct bpf_dynptr *ptr__uninit)
> >> {
> >> struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit;
> >> int err;
> >> @@ -12064,10 +12062,16 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
> >> return 0;
> >> }
>
> [...]
>
> >> +
> >> +static u32 bpf_kfunc_set_skb_remap(const struct bpf_prog *prog, u32 kfunc_id)
> >> +{
> >> + if (kfunc_id != bpf_dynptr_from_skb_list[0])
> >> + return 0;
> >> +
> >> + switch (resolve_prog_type(prog)) {
> >> + /* Program types only with direct read access go here! */
> >> + case BPF_PROG_TYPE_LWT_IN:
> >> + case BPF_PROG_TYPE_LWT_OUT:
> >> + case BPF_PROG_TYPE_LWT_SEG6LOCAL:
> >> + case BPF_PROG_TYPE_SK_REUSEPORT:
> >> + case BPF_PROG_TYPE_FLOW_DISSECTOR:
> >> + case BPF_PROG_TYPE_CGROUP_SKB:
> >> + return bpf_dynptr_from_skb_list[1];
> >> +
> >> + /* Program types with direct read + write access go here! */
> >> + case BPF_PROG_TYPE_SCHED_CLS:
> >> + case BPF_PROG_TYPE_SCHED_ACT:
> >> + case BPF_PROG_TYPE_XDP:
> >> + case BPF_PROG_TYPE_LWT_XMIT:
> >> + case BPF_PROG_TYPE_SK_SKB:
> >> + case BPF_PROG_TYPE_SK_MSG:
> >> + case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> >> + return kfunc_id;
> >> +
> >> + default:
> >> + break;
> >> + }
> >> + return bpf_dynptr_from_skb_list[1];
> >> +}
> >
> > I'd personally prefer the approach we have with BPF helpers, where
> > each program type has a function that handles all helpers (identified
> > by its ID), and then we can use C code sharing to minimize duplication
> > of code.
>
> Different hooks of the same program type, especially struct_ops, may
> not have same access to different kfuncs. Therefore, I am not sure
> whether the approach with helpers can scale in the long term. At the
> moment, we use special_kfunc_[type|set|list] to handle special cases.
> But I am afraid this approach cannot work well with more struct_ops
> and kfuncs.
>
I think we had discussion in the similar vein at last LSF/MM/BPF.
struct_ops is sort of a "meta program type", I don't consider it to be
sufficient by itself. For struct_ops you need to know which exact
struct_ops callback is being considered (that's what would identify
"BPF program type" in the pre-struct_ops world).
But anyways, somehow BPF helpers approach worked across lots of
program types, not sure I see why it wouldn't work for kfuncs
(especially taking into account extending struct_ops with more
detailed "which struct_ops callback" bit).
> >
> > With this approach it seems like we'll have more duplication and we'll
> > need to repeat these program type-based large switches for various
> > small sets of kfuncs, no?
>
> The motivation is to make the verification of kfuncs more modular, so
> that each set of kfuncs handle their verification as much as possible.
>
> I think the code duplication here (bpf_kfunc_set_skb_remap) is not a
> common problem. And we can actually reduce duplication with some
> simple helpers.
>
> Does this make sense?
see above, I'm a bit skeptical, buf proof is in the pudding ;)
>
> Thanks,
> Song
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-01-14 23:41 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-08 22:51 [PATCH v8 bpf-next 0/7] Enable writing xattr from BPF programs Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 1/7] fs/xattr: bpf: Introduce security.bpf. xattr name prefix Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 2/7] selftests/bpf: Extend test fs_kfuncs to cover security.bpf. xattr names Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 3/7] bpf: lsm: Add two more sleepable hooks Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 4/7] bpf: Extend btf_kfunc_id_set to handle kfunc polymorphism Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 5/7] bpf: Use btf_kfunc_id_set.remap logic for bpf_dynptr_from_skb Song Liu
2025-01-09 23:55 ` kernel test robot
2025-01-10 1:08 ` Song Liu
2025-01-10 1:11 ` kernel test robot
2025-01-14 22:37 ` Andrii Nakryiko
2025-01-14 23:03 ` Song Liu
2025-01-14 23:40 ` Andrii Nakryiko
2025-01-08 22:51 ` [PATCH v8 bpf-next 6/7] bpf: fs/xattr: Add BPF kfuncs to set and remove xattrs Song Liu
2025-01-08 22:51 ` [PATCH v8 bpf-next 7/7] selftests/bpf: Test kfuncs that set and remove xattr from BPF programs Song Liu
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).