Add bpf_getxattr
Subsystems (6)
KP Singh
maintainercover Add bpf_getxattr
Foundation for building more complex security policies using the BPF LSM as presented in LSF/MM/BPF:
http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-xattr.pdf
KP Singh (2):
bpf: Implement bpf_getxattr helper
bpf/selftests: Add a selftest for bpf_getxattr
include/uapi/linux/bpf.h | 8 +++ kernel/trace/bpf_trace.c | 26 +++++++++ scripts/bpf_doc.py | 5 ++ tools/include/uapi/linux/bpf.h | 8 +++ .../testing/selftests/bpf/prog_tests/xattr.c | 58 +++++++++++++++++++ tools/testing/selftests/bpf/progs/xattr.c | 34 +++++++++++ 6 files changed, 139 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c create mode 100644 tools/testing/selftests/bpf/progs/xattr.c
Signature · 1 line
2.36.0.550.gb090851708-goog
KP Singh
maintainerPATCH 1/2 bpf: Implement bpf_getxattr helper
LSMs like SELinux store security state in xattrs. bpf_getxattr enables BPF LSM to implement similar functionality. In combination with bpf_local_storage, xattrs can be used to develop more complex security policies.
This helper wraps around vfs_getxattr which can sleep and is, therefore, limited to sleepable programs.
--- include/uapi/linux/bpf.h | 8 ++++++++ kernel/trace/bpf_trace.c | 26 ++++++++++++++++++++++++++ scripts/bpf_doc.py | 5 +++++ tools/include/uapi/linux/bpf.h | 8 ++++++++ 4 files changed, 47 insertions(+)
include/uapi/linux/bpf.h +8 -0
@@ -5172,6 +5172,13 @@ union bpf_attr {* Return * Map value associated to *key* on *cpu*, or **NULL** if no entry * was found or *cpu* is invalid. +* +* long bpf_getxattr(struct user_namespace *mnt_userns, struct dentry *dentry, const char *name, void *value, u64 value_size) +* Description +* Get the *value* of the xattr with the given *name* +* where *value_size* is the size of the *value* buffer. +* Return +* The number of bytes copied into *value*. */ #define __BPF_FUNC_MAPPER (FN ) \ FN(unspec), \
@@ -5370,6 +5377,7 @@ union bpf_attr {FN (ima_file_hash), \FN (kptr_xchg), \FN (map_lookup_percpu_elem), \ +FN (getxattr), \/* */ /* integer value in 'imm' field of BPF_CALL instruction selects which helper
kernel/trace/bpf_trace.c +26 -0
@@ -20,6 +20,7 @@#include <linux/fprobe.h> #include <linux/bsearch.h> #include <linux/sort.h> +#include <linux/xattr.h> #include <net/bpf_sk_storage.h>
@@ -1181,6 +1182,29 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = { .arg1_type = ARG_PTR_TO_CTX, }; +BPF_CALL_5(bpf_getxattr, struct user_namespace *, mnt_userns, struct dentry *, + dentry, void *, name, void *, value, size_t, value_size) +{ +return vfs_getxattr (mnt_userns, dentry, name, value, value_size); +} + +BTF_ID_LIST (bpf_getxattr_btf_ids) +BTF_ID(struct, user_namespace) +BTF_ID (struct, dentry) + +static const struct bpf_func_proto bpf_getxattr_proto = { + .func = bpf_getxattr, + .gpl_only = false, + .ret_type =RET_INTEGER , + .arg1_type =ARG_PTR_TO_BTF_ID , + .arg1_btf_id = &bpf_getxattr_btf_ids[0 ], + .arg2_type =ARG_PTR_TO_BTF_ID , + .arg2_btf_id = &bpf_getxattr_btf_ids[1 ], + .arg3_type =ARG_PTR_TO_CONST_STR , + .arg4_type =ARG_PTR_TO_UNINIT_MEM , + .arg5_type =ARG_CONST_SIZE , +}; +static const struct bpf_func_proto *bpf_tracing_func_proto (enum bpf_func_id func_id,const struct bpf_prog *prog) {
@@ -1304,6 +1328,8 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)return &bpf_find_vma_proto;case BPF_FUNC_trace_vprintk:return bpf_get_trace_vprintk_proto (); +case BPF_FUNC_getxattr: +return prog->aux->sleepable ? &bpf_getxattr_proto :NULL ;default :return bpf_base_func_proto (func_id); }
scripts/bpf_doc.py +5 -0
@@ -633,6 +633,8 @@ class PrinterHelpers(Printer): 'struct socket', 'struct file', 'struct bpf_timer', + 'struct user_namespace', + 'struct dentry', ] known_types = { '...',
@@ -682,6 +684,9 @@ class PrinterHelpers(Printer): 'struct socket', 'struct file', 'struct bpf_timer', + 'struct user_namespace', + 'struct dentry', + } mapped_types = { 'u8': '__u8',
tools/include/uapi/linux/bpf.h +8 -0
@@ -5172,6 +5172,13 @@ union bpf_attr {* Return * Map value associated to *key* on *cpu*, or **NULL** if no entry * was found or *cpu* is invalid. +* +* long bpf_getxattr(struct user_namespace *mnt_userns, struct dentry *dentry, const char *name, void *value, u64 value_size) +* Description +* Get the *value* of the xattr with the given *name* +* where *value_size* is the size of the *value* buffer. +* Return +* The number of bytes copied into *value*. */ #define __BPF_FUNC_MAPPER (FN ) \ FN(unspec), \
@@ -5370,6 +5377,7 @@ union bpf_attr {FN (ima_file_hash), \FN (kptr_xchg), \FN (map_lookup_percpu_elem), \ +FN (getxattr), \/* */ /* integer value in 'imm' field of BPF_CALL instruction selects which helper
Signature · 1 line
2.36.0.550.gb090851708-goog
Re: [PATCH bpf-next 1/2] bpf: Implement bpf_getxattr helper
kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = { · line 1189
Quoted 6 lines · KP Singh
kernel/trace/bpf_trace.c +5
-0+BPF_CALL_5(bpf_getxattr, struct user_namespace *, mnt_userns, struct dentry *, + dentry, void *, name, void *, value, size_t, value_size) +{ +return vfs_getxattr (mnt_userns, dentry, name, value, value_size); +}
It will deadlock in tracing, since it grabs all kinds of locks and calls lsm hooks (potentially calling other bpf progs). It probably should be sleepable only.
Also there is no need to make it uapi.
kfunc is a better interface here.__vfs_getxattr() is probably better too,
since vfs_getxattr() calls xattr_permission which calls a bunch of capable*() which will return "random values" depending on the current task, since it's called from bpf prog.
KP Singh
maintainerRe: [PATCH bpf-next 1/2] bpf: Implement bpf_getxattr helper
Quoted 11 lines · Alexei Starovoitov
kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = { · line 1189
Quoted 6 lines · KP Singh
kernel/trace/bpf_trace.c +5
-0+BPF_CALL_5(bpf_getxattr, struct user_namespace *, mnt_userns, struct dentry *, + dentry, void *, name, void *, value, size_t, value_size) +{ +return vfs_getxattr (mnt_userns, dentry, name, value, value_size); +}It will deadlock in tracing, since it grabs all kinds of locks and calls lsm hooks (potentially calling other bpf progs).
I wonder if we can limit these to just sleepable LSM programs and for sleepable hooks + programs.
Quoted 1 line
It probably should be sleepable only.
Yes, it's currently sleepable only.
Quoted 2 lines
Also there is no need to make it uapi.
kfunc is a better interface here.
Sure, let me try with kfunc, simple wrappers like
these are a good use-case for kfuncs.
Quoted 3 lines
__vfs_getxattr()is probably better too,
sincevfs_getxattr()callsxattr_permissionwhich calls a bunch of capable*() which will return "random values"
Agreed.
Quoted 1 line
depending on the current task, since it's called from bpf prog.
KP Singh
maintainerPATCH 2/2 bpf/selftests: Add a selftest for bpf_getxattr
In reply to KP SinghA simple test that adds an xattr on a copied /bin/ls and reads it back when the copied ls is executed.
--- .../testing/selftests/bpf/prog_tests/xattr.c | 58 +++++++++++++++++++ tools/testing/selftests/bpf/progs/xattr.c | 34 +++++++++++ 2 files changed, 92 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/xattr.c create mode 100644 tools/testing/selftests/bpf/progs/xattr.c
tools/testing/selftests/bpf/prog_tests/xattr.c new file +58 -0
@@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* +* Copyright 2022 Google LLC. +*/ + +#include <test_progs.h> +#include <sys/xattr.h> +#include "xattr.skel.h" + +#define XATTR_NAME "security.bpf" +#define XATTR_VALUE "test_progs" + +static unsigned int duration; + +void test_xattr (void ) +{ +struct xattr *skel =NULL ; +char tmp_dir_path[] ="/tmp/xattrXXXXXX" ; +char tmp_exec_path[64 ]; +char cmd[256 ]; +int err; + +if (CHECK (!mkdtemp (tmp_dir_path),"mkdtemp" , +"unable to create tmpdir: %d\n" , errno)) + goto close_prog; + +snprintf (tmp_exec_path,sizeof (tmp_exec_path),"%s/copy_of_ls" , + tmp_dir_path); +snprintf (cmd,sizeof (cmd),"cp /bin/ls %s" , tmp_exec_path); +if (CHECK_FAIL (system (cmd))) + goto close_prog_rmdir; + +if (CHECK (setxattr (tmp_exec_path,XATTR_NAME ,XATTR_VALUE , +sizeof (XATTR_VALUE ),0 ), +"setxattr" ,"unable to setxattr: %d" , errno)) + goto close_prog_rmdir; + + skel =xattr__open_and_load (); +if (!ASSERT_OK_PTR (skel,"skel_load" )) + goto close_prog_rmdir; + + err =xattr__attach (skel); +if (!ASSERT_OK (err,"xattr__attach failed" )) + goto close_prog_rmdir; + +snprintf (cmd,sizeof (cmd),"%s -l" , tmp_exec_path); +if (CHECK_FAIL (system (cmd))) + goto close_prog_rmdir; + +ASSERT_EQ (skel->bss->result,1 ,"xattr result" ); + +close_prog_rmdir: +snprintf (cmd,sizeof (cmd),"rm -rf %s" , tmp_dir_path); +system (cmd); +close_prog: +xattr__destroy (skel); +}
tools/testing/selftests/bpf/progs/xattr.c new file +34 -0
@@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* +* Copyright 2022 Google LLC. +*/ + +#include "vmlinux.h" +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +char _license[] SEC("license") = "GPL"; + +#define XATTR_NAME "security.bpf" +#define XATTR_VALUE "test_progs" + +__u64 result =0 ; + +SEC("lsm.s/bprm_committed_creds") +void BPF_PROG (bprm_cc ,struct linux_binprm *bprm) +{ +struct task_struct *current =bpf_get_current_task_btf (); +char dir_xattr_value[64 ]; +int xattr_sz; + + xattr_sz =bpf_getxattr (bprm->file->f_path.mnt->mnt_userns, + bprm->file->f_path.dentry,XATTR_NAME , + dir_xattr_value,64 ); + +if (xattr_sz <=0 ) +return ; + +if (!bpf_strncmp (dir_xattr_value,sizeof (XATTR_VALUE ),XATTR_VALUE )) + result =1 ; +}
Signature · 1 line
2.36.0.550.gb090851708-goog