| Message ID | 20250304203123.3935371-3-bboscaccy@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | BPF |
| Headers | show |
| Series | security: Propagate caller information in bpf hooks | expand |
On Tue, Mar 4, 2025 at 12:31 PM Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote: > > The security_bpf LSM hook now contains a boolean parameter specifying > whether an invocation of the bpf syscall originated from within the > kernel. Here, we update the function signature of relevant test > programs to include that new parameter. > > Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com ^^^ The email address is broken. > --- > tools/testing/selftests/bpf/progs/rcu_read_lock.c | 3 ++- > tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c | 4 ++-- > tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++--- > tools/testing/selftests/bpf/progs/test_lookup_key.c | 2 +- > tools/testing/selftests/bpf/progs/test_ptr_untrusted.c | 2 +- > tools/testing/selftests/bpf/progs/test_task_under_cgroup.c | 2 +- > tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c | 2 +- > 7 files changed, 11 insertions(+), 10 deletions(-) It appears you missed a few of these? tools/testing/selftests/bpf/progs/rcu_read_lock.c:SEC("?lsm.s/bpf") tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c:SEC("lsm/bpf") tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("?lsm.s/bpf") tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("?lsm.s/bpf") tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/test_libbpf_get_fd_by_id_opts.c:SEC("lsm/bpf_map") tools/testing/selftests/bpf/progs/test_lookup_key.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/test_ptr_untrusted.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/test_task_under_cgroup.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/token_lsm.c:SEC("lsm/bpf_token_capable") tools/testing/selftests/bpf/progs/token_lsm.c:SEC("lsm/bpf_token_cmd") tools/testing/selftests/bpf/progs/verifier_global_subprogs.c:SEC("?lsm/bpf") tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > > diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c > index ab3a532b7dd6d..f85d0e282f2ae 100644 > --- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c > +++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c > @@ -242,7 +242,8 @@ int inproper_sleepable_helper(void *ctx) > } > > SEC("?lsm.s/bpf") > -int BPF_PROG(inproper_sleepable_kfunc, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(inproper_sleepable_kfunc, int cmd, union bpf_attr *attr, unsigned int size, > + bool is_kernel) > { > struct bpf_key *bkey; > > diff --git a/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c > index 44628865fe1d4..0e741262138f2 100644 > --- a/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c > +++ b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c > @@ -51,13 +51,13 @@ static int bpf_link_create_verify(int cmd) > } > > SEC("lsm/bpf") > -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > return bpf_link_create_verify(cmd); > } > > SEC("lsm.s/bpf") > -int BPF_PROG(lsm_s_run, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(lsm_s_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > return bpf_link_create_verify(cmd); > } > diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c > index cd4d752bd089c..ce36a55ba5b8b 100644 > --- a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c > +++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c > @@ -36,7 +36,7 @@ char _license[] SEC("license") = "GPL"; > > SEC("?lsm.s/bpf") > __failure __msg("cannot pass in dynptr at an offset=-8") > -int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > unsigned long val; > > @@ -46,7 +46,7 @@ int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) > > SEC("?lsm.s/bpf") > __failure __msg("arg#0 expected pointer to stack or const struct bpf_dynptr") > -int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > unsigned long val = 0; > > @@ -55,7 +55,7 @@ int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size) > } > > SEC("lsm.s/bpf") > -int BPF_PROG(dynptr_data_null, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(dynptr_data_null, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > struct bpf_key *trusted_keyring; > struct bpf_dynptr ptr; > diff --git a/tools/testing/selftests/bpf/progs/test_lookup_key.c b/tools/testing/selftests/bpf/progs/test_lookup_key.c > index c73776990ae30..c46077e01a4ca 100644 > --- a/tools/testing/selftests/bpf/progs/test_lookup_key.c > +++ b/tools/testing/selftests/bpf/progs/test_lookup_key.c > @@ -23,7 +23,7 @@ extern struct bpf_key *bpf_lookup_system_key(__u64 id) __ksym; > extern void bpf_key_put(struct bpf_key *key) __ksym; > > SEC("lsm.s/bpf") > -int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > struct bpf_key *bkey; > __u32 pid; > diff --git a/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c b/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c > index 2fdc44e766248..21fce1108a21d 100644 > --- a/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c > +++ b/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c > @@ -7,7 +7,7 @@ > char tp_name[128]; > > SEC("lsm.s/bpf") > -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > switch (cmd) { > case BPF_RAW_TRACEPOINT_OPEN: > diff --git a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c > index 7e750309ce274..18ad24a851c6c 100644 > --- a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c > +++ b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c > @@ -49,7 +49,7 @@ int BPF_PROG(tp_btf_run, struct task_struct *task, u64 clone_flags) > } > > SEC("lsm.s/bpf") > -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > struct cgroup *cgrp = NULL; > struct task_struct *task; > diff --git a/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c > index 12034a73ee2d2..135665f011c7e 100644 > --- a/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c > +++ b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c > @@ -37,7 +37,7 @@ struct { > char _license[] SEC("license") = "GPL"; > > SEC("lsm.s/bpf") > -int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size) > +int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) > { > struct bpf_dynptr data_ptr, sig_ptr; > struct data *data_val; > -- > 2.48.1 >
Song Liu <song@kernel.org> writes: > On Tue, Mar 4, 2025 at 12:31 PM Blaise Boscaccy > <bboscaccy@linux.microsoft.com> wrote: >> >> The security_bpf LSM hook now contains a boolean parameter specifying >> whether an invocation of the bpf syscall originated from within the >> kernel. Here, we update the function signature of relevant test >> programs to include that new parameter. >> >> Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com > ^^^ The email address is broken. > Whoops, appologies, will get that fixed. >> --- >> tools/testing/selftests/bpf/progs/rcu_read_lock.c | 3 ++- >> tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c | 4 ++-- >> tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++--- >> tools/testing/selftests/bpf/progs/test_lookup_key.c | 2 +- >> tools/testing/selftests/bpf/progs/test_ptr_untrusted.c | 2 +- >> tools/testing/selftests/bpf/progs/test_task_under_cgroup.c | 2 +- >> tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c | 2 +- >> 7 files changed, 11 insertions(+), 10 deletions(-) > > It appears you missed a few of these? > Some of these don't require any changes. I ran into this as well while doing a search. These are all accounted for in the patch. > tools/testing/selftests/bpf/progs/rcu_read_lock.c:SEC("?lsm.s/bpf") > tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c:SEC("lsm/bpf") > tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("?lsm.s/bpf") > tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("?lsm.s/bpf") > tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("lsm.s/bpf") security_bpf_map wasn't altered, it can't be called from the kernel. No changes needed. > tools/testing/selftests/bpf/progs/test_libbpf_get_fd_by_id_opts.c:SEC("lsm/bpf_map") These are also all accounted for in the patch. > tools/testing/selftests/bpf/progs/test_lookup_key.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/test_ptr_untrusted.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/test_task_under_cgroup.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c:SEC("lsm.s/bpf") bpf_token_cmd and bpf_token_capabable aren't callable from the kernel, no changes to that hook either currently. > tools/testing/selftests/bpf/progs/token_lsm.c:SEC("lsm/bpf_token_capable") > tools/testing/selftests/bpf/progs/token_lsm.c:SEC("lsm/bpf_token_cmd") This program doesn't take any parameters currently. > tools/testing/selftests/bpf/progs/verifier_global_subprogs.c:SEC("?lsm/bpf") These are all naked calls that don't take any explicit parameters. > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > -blaise >> >> diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c >> index ab3a532b7dd6d..f85d0e282f2ae 100644 >> --- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c >> +++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c >> @@ -242,7 +242,8 @@ int inproper_sleepable_helper(void *ctx) >> } >> >> SEC("?lsm.s/bpf") >> -int BPF_PROG(inproper_sleepable_kfunc, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(inproper_sleepable_kfunc, int cmd, union bpf_attr *attr, unsigned int size, >> + bool is_kernel) >> { >> struct bpf_key *bkey; >> >> diff --git a/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c >> index 44628865fe1d4..0e741262138f2 100644 >> --- a/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c >> +++ b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c >> @@ -51,13 +51,13 @@ static int bpf_link_create_verify(int cmd) >> } >> >> SEC("lsm/bpf") >> -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> return bpf_link_create_verify(cmd); >> } >> >> SEC("lsm.s/bpf") >> -int BPF_PROG(lsm_s_run, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(lsm_s_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> return bpf_link_create_verify(cmd); >> } >> diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c >> index cd4d752bd089c..ce36a55ba5b8b 100644 >> --- a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c >> +++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c >> @@ -36,7 +36,7 @@ char _license[] SEC("license") = "GPL"; >> >> SEC("?lsm.s/bpf") >> __failure __msg("cannot pass in dynptr at an offset=-8") >> -int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> unsigned long val; >> >> @@ -46,7 +46,7 @@ int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) >> >> SEC("?lsm.s/bpf") >> __failure __msg("arg#0 expected pointer to stack or const struct bpf_dynptr") >> -int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> unsigned long val = 0; >> >> @@ -55,7 +55,7 @@ int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size) >> } >> >> SEC("lsm.s/bpf") >> -int BPF_PROG(dynptr_data_null, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(dynptr_data_null, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> struct bpf_key *trusted_keyring; >> struct bpf_dynptr ptr; >> diff --git a/tools/testing/selftests/bpf/progs/test_lookup_key.c b/tools/testing/selftests/bpf/progs/test_lookup_key.c >> index c73776990ae30..c46077e01a4ca 100644 >> --- a/tools/testing/selftests/bpf/progs/test_lookup_key.c >> +++ b/tools/testing/selftests/bpf/progs/test_lookup_key.c >> @@ -23,7 +23,7 @@ extern struct bpf_key *bpf_lookup_system_key(__u64 id) __ksym; >> extern void bpf_key_put(struct bpf_key *key) __ksym; >> >> SEC("lsm.s/bpf") >> -int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> struct bpf_key *bkey; >> __u32 pid; >> diff --git a/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c b/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c >> index 2fdc44e766248..21fce1108a21d 100644 >> --- a/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c >> +++ b/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c >> @@ -7,7 +7,7 @@ >> char tp_name[128]; >> >> SEC("lsm.s/bpf") >> -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> switch (cmd) { >> case BPF_RAW_TRACEPOINT_OPEN: >> diff --git a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c >> index 7e750309ce274..18ad24a851c6c 100644 >> --- a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c >> +++ b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c >> @@ -49,7 +49,7 @@ int BPF_PROG(tp_btf_run, struct task_struct *task, u64 clone_flags) >> } >> >> SEC("lsm.s/bpf") >> -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> struct cgroup *cgrp = NULL; >> struct task_struct *task; >> diff --git a/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c >> index 12034a73ee2d2..135665f011c7e 100644 >> --- a/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c >> +++ b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c >> @@ -37,7 +37,7 @@ struct { >> char _license[] SEC("license") = "GPL"; >> >> SEC("lsm.s/bpf") >> -int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size) >> +int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) >> { >> struct bpf_dynptr data_ptr, sig_ptr; >> struct data *data_val; >> -- >> 2.48.1 >>
On Tue, Mar 4, 2025 at 3:31 PM Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote: > > The security_bpf LSM hook now contains a boolean parameter specifying > whether an invocation of the bpf syscall originated from within the > kernel. Here, we update the function signature of relevant test > programs to include that new parameter. > > Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com > --- > tools/testing/selftests/bpf/progs/rcu_read_lock.c | 3 ++- > tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c | 4 ++-- > tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++--- > tools/testing/selftests/bpf/progs/test_lookup_key.c | 2 +- > tools/testing/selftests/bpf/progs/test_ptr_untrusted.c | 2 +- > tools/testing/selftests/bpf/progs/test_task_under_cgroup.c | 2 +- > tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c | 2 +- > 7 files changed, 11 insertions(+), 10 deletions(-) I see that Song requested that the changes in this patch be split out back in the v3 revision, will that cause git bisect issues if patch 1/2 is applied but patch 2/2 is not, or is there some BPF magic that ensures that the selftests will still run properly?
Paul Moore <paul@paul-moore.com> writes: > On Tue, Mar 4, 2025 at 3:31 PM Blaise Boscaccy > <bboscaccy@linux.microsoft.com> wrote: >> >> The security_bpf LSM hook now contains a boolean parameter specifying >> whether an invocation of the bpf syscall originated from within the >> kernel. Here, we update the function signature of relevant test >> programs to include that new parameter. >> >> Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com >> --- >> tools/testing/selftests/bpf/progs/rcu_read_lock.c | 3 ++- >> tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c | 4 ++-- >> tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++--- >> tools/testing/selftests/bpf/progs/test_lookup_key.c | 2 +- >> tools/testing/selftests/bpf/progs/test_ptr_untrusted.c | 2 +- >> tools/testing/selftests/bpf/progs/test_task_under_cgroup.c | 2 +- >> tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c | 2 +- >> 7 files changed, 11 insertions(+), 10 deletions(-) > > I see that Song requested that the changes in this patch be split out > back in the v3 revision, will that cause git bisect issues if patch > 1/2 is applied but patch 2/2 is not, or is there some BPF magic that > ensures that the selftests will still run properly? > So there isn't any type checking in the bpf program's function arguments against the LSM hook definitions, so it shouldn't cause any build issues. To the best of my knowledge, the new is_kernel boolean flag will end up living in r3. None of the current tests reference that parameter, so if we bisected and ended up on the previous commit, the bpf test programs would in a worst-case scenario simply clobber that register, which shouldn't effect any test outcomes unless a test program was somehow dependent on an uninitialized value in a scratch register. -blaise > -- > paul-moore.com
On Tue, Mar 4, 2025 at 8:26 PM Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote: > Paul Moore <paul@paul-moore.com> writes: > > On Tue, Mar 4, 2025 at 3:31 PM Blaise Boscaccy > > <bboscaccy@linux.microsoft.com> wrote: > >> > >> The security_bpf LSM hook now contains a boolean parameter specifying > >> whether an invocation of the bpf syscall originated from within the > >> kernel. Here, we update the function signature of relevant test > >> programs to include that new parameter. > >> > >> Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com > >> --- > >> tools/testing/selftests/bpf/progs/rcu_read_lock.c | 3 ++- > >> tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c | 4 ++-- > >> tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++--- > >> tools/testing/selftests/bpf/progs/test_lookup_key.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_ptr_untrusted.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_task_under_cgroup.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c | 2 +- > >> 7 files changed, 11 insertions(+), 10 deletions(-) > > > > I see that Song requested that the changes in this patch be split out > > back in the v3 revision, will that cause git bisect issues if patch > > 1/2 is applied but patch 2/2 is not, or is there some BPF magic that > > ensures that the selftests will still run properly? > > > > So there isn't any type checking in the bpf program's function > arguments against the LSM hook definitions, so it shouldn't cause any > build issues. To the best of my knowledge, the new is_kernel boolean > flag will end up living in r3. None of the current tests reference > that parameter, so if we bisected and ended up on the previous commit, > the bpf test programs would in a worst-case scenario simply clobber that > register, which shouldn't effect any test outcomes unless a test program > was somehow dependent on an uninitialized value in a scratch register. Esh. With that in mind, I'd argue that the two patches really should just be one patch as you did before. The patches are both pretty small and obviously related so it really shouldn't be an issue. However, since we need this patchset in order to properly implement BPF signature verification I'm not going to make a fuss if Song feels strongly that the selftest changes should be split into their own patch.
On Tue, Mar 4, 2025 at 4:36 PM Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote: > > Song Liu <song@kernel.org> writes: > > > On Tue, Mar 4, 2025 at 12:31 PM Blaise Boscaccy > > <bboscaccy@linux.microsoft.com> wrote: > >> > >> The security_bpf LSM hook now contains a boolean parameter specifying > >> whether an invocation of the bpf syscall originated from within the > >> kernel. Here, we update the function signature of relevant test > >> programs to include that new parameter. > >> > >> Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com > > ^^^ The email address is broken. > > > > Whoops, appologies, will get that fixed. > > >> --- > >> tools/testing/selftests/bpf/progs/rcu_read_lock.c | 3 ++- > >> tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c | 4 ++-- > >> tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++--- > >> tools/testing/selftests/bpf/progs/test_lookup_key.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_ptr_untrusted.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_task_under_cgroup.c | 2 +- > >> tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c | 2 +- > >> 7 files changed, 11 insertions(+), 10 deletions(-) > > > > It appears you missed a few of these? > > > > Some of these don't require any changes. I ran into this as well while doing a > search. > > These are all accounted for in the patch. > > tools/testing/selftests/bpf/progs/rcu_read_lock.c:SEC("?lsm.s/bpf") > > tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c:SEC("lsm/bpf") > > tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("?lsm.s/bpf") > > tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("?lsm.s/bpf") > > tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c:SEC("lsm.s/bpf") > > security_bpf_map wasn't altered, it can't be called from the kernel. No > changes needed. > > tools/testing/selftests/bpf/progs/test_libbpf_get_fd_by_id_opts.c:SEC("lsm/bpf_map") > > These are also all accounted for in the patch. > > tools/testing/selftests/bpf/progs/test_lookup_key.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/test_ptr_untrusted.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/test_task_under_cgroup.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c:SEC("lsm.s/bpf") > > bpf_token_cmd and bpf_token_capabable aren't callable from the kernel, > no changes to that hook either currently. > > > tools/testing/selftests/bpf/progs/token_lsm.c:SEC("lsm/bpf_token_capable") > > tools/testing/selftests/bpf/progs/token_lsm.c:SEC("lsm/bpf_token_cmd") > > > This program doesn't take any parameters currently. > > tools/testing/selftests/bpf/progs/verifier_global_subprogs.c:SEC("?lsm/bpf") > > These are all naked calls that don't take any explicit parameters. > > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") > > tools/testing/selftests/bpf/progs/verifier_ref_tracking.c:SEC("lsm.s/bpf") Thanks for the explanation. I think we can keep this part as-is. Song
On Tue, Mar 4, 2025 at 6:14 PM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Mar 4, 2025 at 8:26 PM Blaise Boscaccy > <bboscaccy@linux.microsoft.com> wrote: > > Paul Moore <paul@paul-moore.com> writes: > > > On Tue, Mar 4, 2025 at 3:31 PM Blaise Boscaccy > > > <bboscaccy@linux.microsoft.com> wrote: > > >> > > >> The security_bpf LSM hook now contains a boolean parameter specifying > > >> whether an invocation of the bpf syscall originated from within the > > >> kernel. Here, we update the function signature of relevant test > > >> programs to include that new parameter. > > >> > > >> Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com > > >> --- > > >> tools/testing/selftests/bpf/progs/rcu_read_lock.c | 3 ++- > > >> tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c | 4 ++-- > > >> tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++--- > > >> tools/testing/selftests/bpf/progs/test_lookup_key.c | 2 +- > > >> tools/testing/selftests/bpf/progs/test_ptr_untrusted.c | 2 +- > > >> tools/testing/selftests/bpf/progs/test_task_under_cgroup.c | 2 +- > > >> tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c | 2 +- > > >> 7 files changed, 11 insertions(+), 10 deletions(-) > > > > > > I see that Song requested that the changes in this patch be split out > > > back in the v3 revision, will that cause git bisect issues if patch > > > 1/2 is applied but patch 2/2 is not, or is there some BPF magic that > > > ensures that the selftests will still run properly? > > > > > > > So there isn't any type checking in the bpf program's function > > arguments against the LSM hook definitions, so it shouldn't cause any > > build issues. To the best of my knowledge, the new is_kernel boolean > > flag will end up living in r3. None of the current tests reference > > that parameter, so if we bisected and ended up on the previous commit, > > the bpf test programs would in a worst-case scenario simply clobber that > > register, which shouldn't effect any test outcomes unless a test program > > was somehow dependent on an uninitialized value in a scratch register. > > Esh. With that in mind, I'd argue that the two patches really should > just be one patch as you did before. The patches are both pretty > small and obviously related so it really shouldn't be an issue. > > However, since we need this patchset in order to properly implement > BPF signature verification I'm not going to make a fuss if Song feels > strongly that the selftest changes should be split into their own > patch. On second thought, I think it makes sense to merge the two patches. Blasie, please update 1/2 based on Paul's comment, merge the two patches, and resend. You can keep my Acked-by. Do we need this in the LSM tree before the upcoming merge window? If not, we would prefer to carry it in bpf-next. Thanks, Song
On Tue, Mar 4, 2025 at 10:32 PM Song Liu <song@kernel.org> wrote: > On Tue, Mar 4, 2025 at 6:14 PM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Mar 4, 2025 at 8:26 PM Blaise Boscaccy > > <bboscaccy@linux.microsoft.com> wrote: > > > Paul Moore <paul@paul-moore.com> writes: > > > > On Tue, Mar 4, 2025 at 3:31 PM Blaise Boscaccy > > > > <bboscaccy@linux.microsoft.com> wrote: ... > Do we need this in the LSM tree before the upcoming merge window? > If not, we would prefer to carry it in bpf-next. As long as we can send this up to Linus during the upcoming merge window I'll be happy; if you feel strongly and want to take it via the BPF tree, that's fine by me. I'm currently helping someone draft a patchset to implement the LSM/SELinux access control LSM callbacks for the BPF tokens and I'm also working on a fix for the LSM framework initialization code, both efforts may land in a development tree during the next dev cycle and may cause a merge conflict with Blaise's changes. Not that a merge conflict is a terrible thing that we can't work around, but if we can avoid it I'd be much happier :) Please do make the /is_kernel/kernel/ change I mentioned in patch 1/2, and feel free to keep my ACK from this patchset revision. Thanks everyone!
On Wed, Mar 5, 2025 at 8:12 AM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Mar 4, 2025 at 10:32 PM Song Liu <song@kernel.org> wrote: > > On Tue, Mar 4, 2025 at 6:14 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Tue, Mar 4, 2025 at 8:26 PM Blaise Boscaccy > > > <bboscaccy@linux.microsoft.com> wrote: > > > > Paul Moore <paul@paul-moore.com> writes: > > > > > On Tue, Mar 4, 2025 at 3:31 PM Blaise Boscaccy > > > > > <bboscaccy@linux.microsoft.com> wrote: > > ... > > > Do we need this in the LSM tree before the upcoming merge window? > > If not, we would prefer to carry it in bpf-next. > > As long as we can send this up to Linus during the upcoming merge > window I'll be happy; if you feel strongly and want to take it via the > BPF tree, that's fine by me. I'm currently helping someone draft a > patchset to implement the LSM/SELinux access control LSM callbacks for > the BPF tokens and I'm also working on a fix for the LSM framework > initialization code, both efforts may land in a development tree > during the next dev cycle and may cause a merge conflict with Blaise's > changes. Not that a merge conflict is a terrible thing that we can't > work around, but if we can avoid it I'd be much happier :) > > Please do make the /is_kernel/kernel/ change I mentioned in patch 1/2, > and feel free to keep my ACK from this patchset revision. My preference is to go via bpf-next, since changes are bigger on bpf side than on lsm side. Re: selftest. Why change them at all if 'bool kernel' attribute is unused ? Addition of the attr should be backward compatible change, so all tests should still pass as-is. You probably should add a new test where 'kernel' arg is actually used for something. That would be patch 2.
On Wed, Mar 5, 2025 at 9:08 AM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: [...] > My preference is to go via bpf-next, since changes are bigger > on bpf side than on lsm side. > > Re: selftest. > > Why change them at all if 'bool kernel' attribute is unused ? > Addition of the attr should be backward compatible change, > so all tests should still pass as-is. I was thinking of keeping the argument list in the selftests up to date, so that the users can use selftests as examples when writing their BPF programs. OTOH, with the "bool kernel" at the end of the argument list, it is backward compatible. > You probably should add a new test where 'kernel' arg is actually > used for something. That would be patch 2. +1. This is a great idea. Thanks, Song
diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c index ab3a532b7dd6d..f85d0e282f2ae 100644 --- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c +++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c @@ -242,7 +242,8 @@ int inproper_sleepable_helper(void *ctx) } SEC("?lsm.s/bpf") -int BPF_PROG(inproper_sleepable_kfunc, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(inproper_sleepable_kfunc, int cmd, union bpf_attr *attr, unsigned int size, + bool is_kernel) { struct bpf_key *bkey; diff --git a/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c index 44628865fe1d4..0e741262138f2 100644 --- a/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c +++ b/tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c @@ -51,13 +51,13 @@ static int bpf_link_create_verify(int cmd) } SEC("lsm/bpf") -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { return bpf_link_create_verify(cmd); } SEC("lsm.s/bpf") -int BPF_PROG(lsm_s_run, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(lsm_s_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { return bpf_link_create_verify(cmd); } diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c index cd4d752bd089c..ce36a55ba5b8b 100644 --- a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c +++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c @@ -36,7 +36,7 @@ char _license[] SEC("license") = "GPL"; SEC("?lsm.s/bpf") __failure __msg("cannot pass in dynptr at an offset=-8") -int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { unsigned long val; @@ -46,7 +46,7 @@ int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) SEC("?lsm.s/bpf") __failure __msg("arg#0 expected pointer to stack or const struct bpf_dynptr") -int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { unsigned long val = 0; @@ -55,7 +55,7 @@ int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size) } SEC("lsm.s/bpf") -int BPF_PROG(dynptr_data_null, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(dynptr_data_null, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { struct bpf_key *trusted_keyring; struct bpf_dynptr ptr; diff --git a/tools/testing/selftests/bpf/progs/test_lookup_key.c b/tools/testing/selftests/bpf/progs/test_lookup_key.c index c73776990ae30..c46077e01a4ca 100644 --- a/tools/testing/selftests/bpf/progs/test_lookup_key.c +++ b/tools/testing/selftests/bpf/progs/test_lookup_key.c @@ -23,7 +23,7 @@ extern struct bpf_key *bpf_lookup_system_key(__u64 id) __ksym; extern void bpf_key_put(struct bpf_key *key) __ksym; SEC("lsm.s/bpf") -int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { struct bpf_key *bkey; __u32 pid; diff --git a/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c b/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c index 2fdc44e766248..21fce1108a21d 100644 --- a/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c +++ b/tools/testing/selftests/bpf/progs/test_ptr_untrusted.c @@ -7,7 +7,7 @@ char tp_name[128]; SEC("lsm.s/bpf") -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { switch (cmd) { case BPF_RAW_TRACEPOINT_OPEN: diff --git a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c index 7e750309ce274..18ad24a851c6c 100644 --- a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c +++ b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c @@ -49,7 +49,7 @@ int BPF_PROG(tp_btf_run, struct task_struct *task, u64 clone_flags) } SEC("lsm.s/bpf") -int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { struct cgroup *cgrp = NULL; struct task_struct *task; diff --git a/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c index 12034a73ee2d2..135665f011c7e 100644 --- a/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c +++ b/tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c @@ -37,7 +37,7 @@ struct { char _license[] SEC("license") = "GPL"; SEC("lsm.s/bpf") -int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size) +int BPF_PROG(bpf, int cmd, union bpf_attr *attr, unsigned int size, bool is_kernel) { struct bpf_dynptr data_ptr, sig_ptr; struct data *data_val;
The security_bpf LSM hook now contains a boolean parameter specifying whether an invocation of the bpf syscall originated from within the kernel. Here, we update the function signature of relevant test programs to include that new parameter. Signed-off-by: Blaise Boscaccy bboscaccy@linux.microsoft.com --- tools/testing/selftests/bpf/progs/rcu_read_lock.c | 3 ++- tools/testing/selftests/bpf/progs/test_cgroup1_hierarchy.c | 4 ++-- tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c | 6 +++--- tools/testing/selftests/bpf/progs/test_lookup_key.c | 2 +- tools/testing/selftests/bpf/progs/test_ptr_untrusted.c | 2 +- tools/testing/selftests/bpf/progs/test_task_under_cgroup.c | 2 +- tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c | 2 +- 7 files changed, 11 insertions(+), 10 deletions(-)