* [PATCH bpf-next v2 0/2] Sleepable LSM Hooks
@ 2020-11-12 20:03 KP Singh
2020-11-12 20:03 ` [PATCH bpf-next v2 1/2] bpf: Augment the set of sleepable LSM hooks KP Singh
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: KP Singh @ 2020-11-12 20:03 UTC (permalink / raw)
To: linux-kernel, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Jann Horn, Hao Luo, Florent Revest, Brendan Jackman
From: KP Singh <kpsingh@google.com>
# v1 -> v2
* Fixed typos and formatting errors.
* Added Andrii's ack.
KP Singh (2):
bpf: Augment the set of sleepable LSM hooks
bpf: Expose bpf_d_path helper to sleepable LSM hooks
include/linux/bpf_lsm.h | 7 +++
kernel/bpf/bpf_lsm.c | 121 +++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 16 +-----
kernel/trace/bpf_trace.c | 7 ++-
4 files changed, 135 insertions(+), 16 deletions(-)
--
2.29.2.222.g5d2a92d10f8-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH bpf-next v2 1/2] bpf: Augment the set of sleepable LSM hooks 2020-11-12 20:03 [PATCH bpf-next v2 0/2] Sleepable LSM Hooks KP Singh @ 2020-11-12 20:03 ` KP Singh 2020-11-12 22:35 ` Daniel Borkmann 2020-11-12 20:03 ` [PATCH bpf-next v2 2/2] bpf: Expose bpf_d_path helper to " KP Singh 2020-11-12 20:29 ` [PATCH bpf-next v2 0/2] Sleepable LSM Hooks KP Singh 2 siblings, 1 reply; 6+ messages in thread From: KP Singh @ 2020-11-12 20:03 UTC (permalink / raw) To: linux-kernel, bpf Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Jann Horn, Hao Luo, Florent Revest, Brendan Jackman From: KP Singh <kpsingh@google.com> Update the set of sleepable hooks with the ones that do not trigger a warning with might_fault() when exercised with the correct kernel config options enabled, i.e. DEBUG_ATOMIC_SLEEP=y LOCKDEP=y PROVE_LOCKING=y This means that a sleepable LSM eBPF program can be attached to these LSM hooks. A new helper method bpf_lsm_is_sleepable_hook is added and the set is maintained locally in bpf_lsm.c A comment is added about the list of LSM hooks that have been observed to be called from softirqs, atomic contexts, or the ones that can trigger pagefaults and thus should not be added to this list. Signed-off-by: KP Singh <kpsingh@google.com> --- include/linux/bpf_lsm.h | 7 +++ kernel/bpf/bpf_lsm.c | 121 ++++++++++++++++++++++++++++++++++++++++ kernel/bpf/verifier.c | 16 +----- 3 files changed, 129 insertions(+), 15 deletions(-) diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h index 73226181b744..0d1c33ace398 100644 --- a/include/linux/bpf_lsm.h +++ b/include/linux/bpf_lsm.h @@ -27,6 +27,8 @@ extern struct lsm_blob_sizes bpf_lsm_blob_sizes; int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, const struct bpf_prog *prog); +bool bpf_lsm_is_sleepable_hook(u32 btf_id); + static inline struct bpf_storage_blob *bpf_inode( const struct inode *inode) { @@ -54,6 +56,11 @@ void bpf_task_storage_free(struct task_struct *task); #else /* !CONFIG_BPF_LSM */ +static inline bool bpf_lsm_is_sleepable_hook(u32 btf_id) +{ + return false; +} + static inline int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, const struct bpf_prog *prog) { diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c index e92c51bebb47..47e25da9e8bb 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -13,6 +13,7 @@ #include <linux/bpf_verifier.h> #include <net/bpf_sk_storage.h> #include <linux/bpf_local_storage.h> +#include <linux/btf_ids.h> /* For every LSM hook that allows attachment of BPF programs, declare a nop * function where a BPF program can be attached. @@ -72,6 +73,126 @@ bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) } } +/* The set of hooks which are called without pagefaults disabled and are allowed + * to "sleep" and thus can be used for sleeable BPF programs. + * + * There are some hooks which have been observed to be called from a + * non-sleepable context and should not be added to this set: + * + * bpf_lsm_bpf_prog_free_security + * bpf_lsm_capable + * bpf_lsm_cred_free + * bpf_lsm_d_instantiate + * bpf_lsm_file_alloc_security + * bpf_lsm_file_mprotect + * bpf_lsm_file_send_sigiotask + * bpf_lsm_inet_conn_request + * bpf_lsm_inet_csk_clone + * bpf_lsm_inode_alloc_security + * bpf_lsm_inode_follow_link + * bpf_lsm_inode_permission + * bpf_lsm_key_permission + * bpf_lsm_locked_down + * bpf_lsm_mmap_addr + * bpf_lsm_perf_event_read + * bpf_lsm_ptrace_access_check + * bpf_lsm_req_classify_flow + * bpf_lsm_sb_free_security + * bpf_lsm_sk_alloc_security + * bpf_lsm_sk_clone_security + * bpf_lsm_sk_free_security + * bpf_lsm_sk_getsecid + * bpf_lsm_socket_sock_rcv_skb + * bpf_lsm_sock_graft + * bpf_lsm_task_free + * bpf_lsm_task_getioprio + * bpf_lsm_task_getscheduler + * bpf_lsm_task_kill + * bpf_lsm_task_setioprio + * bpf_lsm_task_setnice + * bpf_lsm_task_setpgid + * bpf_lsm_task_setrlimit + * bpf_lsm_unix_may_send + * bpf_lsm_unix_stream_connect + * bpf_lsm_vm_enough_memory + */ +BTF_SET_START(sleepable_lsm_hooks) +BTF_ID(func, bpf_lsm_bpf) +BTF_ID(func, bpf_lsm_bpf_map) +BTF_ID(func, bpf_lsm_bpf_map_alloc_security) +BTF_ID(func, bpf_lsm_bpf_map_free_security) +BTF_ID(func, bpf_lsm_bpf_prog) +BTF_ID(func, bpf_lsm_bprm_check_security) +BTF_ID(func, bpf_lsm_bprm_committed_creds) +BTF_ID(func, bpf_lsm_bprm_committing_creds) +BTF_ID(func, bpf_lsm_bprm_creds_for_exec) +BTF_ID(func, bpf_lsm_bprm_creds_from_file) +BTF_ID(func, bpf_lsm_capget) +BTF_ID(func, bpf_lsm_capset) +BTF_ID(func, bpf_lsm_cred_prepare) +BTF_ID(func, bpf_lsm_file_ioctl) +BTF_ID(func, bpf_lsm_file_lock) +BTF_ID(func, bpf_lsm_file_open) +BTF_ID(func, bpf_lsm_file_receive) +BTF_ID(func, bpf_lsm_inet_conn_established) +BTF_ID(func, bpf_lsm_inode_create) +BTF_ID(func, bpf_lsm_inode_free_security) +BTF_ID(func, bpf_lsm_inode_getattr) +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_readlink) +BTF_ID(func, bpf_lsm_inode_rename) +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_symlink) +BTF_ID(func, bpf_lsm_inode_unlink) +BTF_ID(func, bpf_lsm_kernel_module_request) +BTF_ID(func, bpf_lsm_kernfs_init_security) +BTF_ID(func, bpf_lsm_key_free) +BTF_ID(func, bpf_lsm_mmap_file) +BTF_ID(func, bpf_lsm_netlink_send) +BTF_ID(func, bpf_lsm_path_notify) +BTF_ID(func, bpf_lsm_release_secctx) +BTF_ID(func, bpf_lsm_sb_alloc_security) +BTF_ID(func, bpf_lsm_sb_eat_lsm_opts) +BTF_ID(func, bpf_lsm_sb_kern_mount) +BTF_ID(func, bpf_lsm_sb_mount) +BTF_ID(func, bpf_lsm_sb_remount) +BTF_ID(func, bpf_lsm_sb_set_mnt_opts) +BTF_ID(func, bpf_lsm_sb_show_options) +BTF_ID(func, bpf_lsm_sb_statfs) +BTF_ID(func, bpf_lsm_sb_umount) +BTF_ID(func, bpf_lsm_settime) +BTF_ID(func, bpf_lsm_socket_accept) +BTF_ID(func, bpf_lsm_socket_bind) +BTF_ID(func, bpf_lsm_socket_connect) +BTF_ID(func, bpf_lsm_socket_create) +BTF_ID(func, bpf_lsm_socket_getpeername) +BTF_ID(func, bpf_lsm_socket_getpeersec_dgram) +BTF_ID(func, bpf_lsm_socket_getsockname) +BTF_ID(func, bpf_lsm_socket_getsockopt) +BTF_ID(func, bpf_lsm_socket_listen) +BTF_ID(func, bpf_lsm_socket_post_create) +BTF_ID(func, bpf_lsm_socket_recvmsg) +BTF_ID(func, bpf_lsm_socket_sendmsg) +BTF_ID(func, bpf_lsm_socket_shutdown) +BTF_ID(func, bpf_lsm_socket_socketpair) +BTF_ID(func, bpf_lsm_syslog) +BTF_ID(func, bpf_lsm_task_alloc) +BTF_ID(func, bpf_lsm_task_getsecid) +BTF_ID(func, bpf_lsm_task_prctl) +BTF_ID(func, bpf_lsm_task_setscheduler) +BTF_ID(func, bpf_lsm_task_to_inode) +BTF_SET_END(sleepable_lsm_hooks) + +bool bpf_lsm_is_sleepable_hook(u32 btf_id) +{ + return btf_id_set_contains(&sleepable_lsm_hooks, btf_id); +} + const struct bpf_prog_ops lsm_prog_ops = { }; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 10da26e55130..364ec1958c85 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -11477,20 +11477,6 @@ static int check_attach_modify_return(unsigned long addr, const char *func_name) return -EINVAL; } -/* non exhaustive list of sleepable bpf_lsm_*() functions */ -BTF_SET_START(btf_sleepable_lsm_hooks) -#ifdef CONFIG_BPF_LSM -BTF_ID(func, bpf_lsm_bprm_committed_creds) -#else -BTF_ID_UNUSED -#endif -BTF_SET_END(btf_sleepable_lsm_hooks) - -static int check_sleepable_lsm_hook(u32 btf_id) -{ - return btf_id_set_contains(&btf_sleepable_lsm_hooks, btf_id); -} - /* list of non-sleepable functions that are otherwise on * ALLOW_ERROR_INJECTION list */ @@ -11712,7 +11698,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, /* LSM progs check that they are attached to bpf_lsm_*() funcs. * Only some of them are sleepable. */ - if (check_sleepable_lsm_hook(btf_id)) + if (bpf_lsm_is_sleepable_hook(btf_id)) ret = 0; break; default: -- 2.29.2.222.g5d2a92d10f8-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: Augment the set of sleepable LSM hooks 2020-11-12 20:03 ` [PATCH bpf-next v2 1/2] bpf: Augment the set of sleepable LSM hooks KP Singh @ 2020-11-12 22:35 ` Daniel Borkmann 2020-11-12 23:59 ` KP Singh 0 siblings, 1 reply; 6+ messages in thread From: Daniel Borkmann @ 2020-11-12 22:35 UTC (permalink / raw) To: KP Singh, linux-kernel, bpf Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Jann Horn, Hao Luo, Florent Revest, Brendan Jackman On 11/12/20 9:03 PM, KP Singh wrote: > From: KP Singh <kpsingh@google.com> > > Update the set of sleepable hooks with the ones that do not trigger > a warning with might_fault() when exercised with the correct kernel > config options enabled, i.e. > > DEBUG_ATOMIC_SLEEP=y > LOCKDEP=y > PROVE_LOCKING=y > > This means that a sleepable LSM eBPF program can be attached to these > LSM hooks. A new helper method bpf_lsm_is_sleepable_hook is added and > the set is maintained locally in bpf_lsm.c > > A comment is added about the list of LSM hooks that have been observed > to be called from softirqs, atomic contexts, or the ones that can > trigger pagefaults and thus should not be added to this list. > [...] > > +/* The set of hooks which are called without pagefaults disabled and are allowed > + * to "sleep" and thus can be used for sleeable BPF programs. > + * > + * There are some hooks which have been observed to be called from a > + * non-sleepable context and should not be added to this set: > + * > + * bpf_lsm_bpf_prog_free_security > + * bpf_lsm_capable > + * bpf_lsm_cred_free > + * bpf_lsm_d_instantiate > + * bpf_lsm_file_alloc_security > + * bpf_lsm_file_mprotect > + * bpf_lsm_file_send_sigiotask > + * bpf_lsm_inet_conn_request > + * bpf_lsm_inet_csk_clone > + * bpf_lsm_inode_alloc_security > + * bpf_lsm_inode_follow_link > + * bpf_lsm_inode_permission > + * bpf_lsm_key_permission > + * bpf_lsm_locked_down > + * bpf_lsm_mmap_addr > + * bpf_lsm_perf_event_read > + * bpf_lsm_ptrace_access_check > + * bpf_lsm_req_classify_flow > + * bpf_lsm_sb_free_security > + * bpf_lsm_sk_alloc_security > + * bpf_lsm_sk_clone_security > + * bpf_lsm_sk_free_security > + * bpf_lsm_sk_getsecid > + * bpf_lsm_socket_sock_rcv_skb > + * bpf_lsm_sock_graft > + * bpf_lsm_task_free > + * bpf_lsm_task_getioprio > + * bpf_lsm_task_getscheduler > + * bpf_lsm_task_kill > + * bpf_lsm_task_setioprio > + * bpf_lsm_task_setnice > + * bpf_lsm_task_setpgid > + * bpf_lsm_task_setrlimit > + * bpf_lsm_unix_may_send > + * bpf_lsm_unix_stream_connect > + * bpf_lsm_vm_enough_memory > + */ I think this is very useful info. I was wondering whether it would make sense to annotate these more closely to the code so there's less chance this info becomes stale? Maybe something like below, not sure ... issue is if you would just place a cant_sleep() in there it might be wrong since this should just document that it can be invoked from non-sleepable context but it might not have to. diff --git a/security/security.c b/security/security.c index a28045dc9e7f..7899bf32cdaa 100644 --- a/security/security.c +++ b/security/security.c @@ -94,6 +94,11 @@ static __initdata bool debug; pr_info(__VA_ARGS__); \ } while (0) +/* + * Placeholder for now to document that hook implementation cannot sleep + * since it could potentially be called from non-sleepable context, too. + */ +#define hook_cant_sleep() do { } while (0) + static bool __init is_enabled(struct lsm_info *lsm) { if (!lsm->enabled) @@ -2522,6 +2527,7 @@ void security_bpf_map_free(struct bpf_map *map) } void security_bpf_prog_free(struct bpf_prog_aux *aux) { + hook_cant_sleep(); call_void_hook(bpf_prog_free_security, aux); } #endif /* CONFIG_BPF_SYSCALL */ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: Augment the set of sleepable LSM hooks 2020-11-12 22:35 ` Daniel Borkmann @ 2020-11-12 23:59 ` KP Singh 0 siblings, 0 replies; 6+ messages in thread From: KP Singh @ 2020-11-12 23:59 UTC (permalink / raw) To: Daniel Borkmann Cc: open list, bpf, Alexei Starovoitov, Martin KaFai Lau, Song Liu, Jann Horn, Hao Luo, Florent Revest, Brendan Jackman On Thu, Nov 12, 2020 at 11:35 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 11/12/20 9:03 PM, KP Singh wrote: > > From: KP Singh <kpsingh@google.com> > > > > Update the set of sleepable hooks with the ones that do not trigger > > a warning with might_fault() when exercised with the correct kernel > > config options enabled, i.e. [...] > > I think this is very useful info. I was wondering whether it would make sense > to annotate these more closely to the code so there's less chance this info > becomes stale? Maybe something like below, not sure ... issue is if you would > just place a cant_sleep() in there it might be wrong since this should just > document that it can be invoked from non-sleepable context but it might not > have to. Indeed, this is why I did not make an explicit cant_sleep() call for these hooks in __bpf_prog_enter (with a change in the signature to pass struct *prog). > diff --git a/security/security.c b/security/security.c > index a28045dc9e7f..7899bf32cdaa 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -94,6 +94,11 @@ static __initdata bool debug; > pr_info(__VA_ARGS__); \ > } while (0) > > +/* > + * Placeholder for now to document that hook implementation cannot sleep > + * since it could potentially be called from non-sleepable context, too. > + */ > +#define hook_cant_sleep() do { } while (0) Good idea! At the very least, we can update the comments in lsm_hooks.h which already mention some of the LSM hooks as being called from non-sleepable contexts. I will remove this comment, send a separate patch to security folks and respin these patches. -KP > + > static bool __init is_enabled(struct lsm_info *lsm) > { > if (!lsm->enabled) > @@ -2522,6 +2527,7 @@ void security_bpf_map_free(struct bpf_map *map) > } > void security_bpf_prog_free(struct bpf_prog_aux *aux) > { > + hook_cant_sleep(); > call_void_hook(bpf_prog_free_security, aux); > } > #endif /* CONFIG_BPF_SYSCALL */ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next v2 2/2] bpf: Expose bpf_d_path helper to sleepable LSM hooks 2020-11-12 20:03 [PATCH bpf-next v2 0/2] Sleepable LSM Hooks KP Singh 2020-11-12 20:03 ` [PATCH bpf-next v2 1/2] bpf: Augment the set of sleepable LSM hooks KP Singh @ 2020-11-12 20:03 ` KP Singh 2020-11-12 20:29 ` [PATCH bpf-next v2 0/2] Sleepable LSM Hooks KP Singh 2 siblings, 0 replies; 6+ messages in thread From: KP Singh @ 2020-11-12 20:03 UTC (permalink / raw) To: linux-kernel, bpf Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Jann Horn, Hao Luo, Florent Revest, Brendan Jackman From: KP Singh <kpsingh@google.com> Sleepable hooks are never called from an NMI/interrupt context, so it is safe to use the bpf_d_path helper in LSM programs attaching to these hooks. The helper is not restricted to sleepable programs and merely uses the list of sleeable hooks as the initial subset of LSM hooks where it can be used. Signed-off-by: KP Singh <kpsingh@google.com> --- kernel/trace/bpf_trace.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index e4515b0f62a8..eab1af02c90d 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -16,6 +16,7 @@ #include <linux/syscalls.h> #include <linux/error-injection.h> #include <linux/btf_ids.h> +#include <linux/bpf_lsm.h> #include <uapi/linux/bpf.h> #include <uapi/linux/btf.h> @@ -1178,7 +1179,11 @@ BTF_SET_END(btf_allowlist_d_path) static bool bpf_d_path_allowed(const struct bpf_prog *prog) { - return btf_id_set_contains(&btf_allowlist_d_path, prog->aux->attach_btf_id); + if (prog->type == BPF_PROG_TYPE_LSM) + return bpf_lsm_is_sleepable_hook(prog->aux->attach_btf_id); + + return btf_id_set_contains(&btf_allowlist_d_path, + prog->aux->attach_btf_id); } BTF_ID_LIST_SINGLE(bpf_d_path_btf_ids, struct, path) -- 2.29.2.222.g5d2a92d10f8-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 0/2] Sleepable LSM Hooks 2020-11-12 20:03 [PATCH bpf-next v2 0/2] Sleepable LSM Hooks KP Singh 2020-11-12 20:03 ` [PATCH bpf-next v2 1/2] bpf: Augment the set of sleepable LSM hooks KP Singh 2020-11-12 20:03 ` [PATCH bpf-next v2 2/2] bpf: Expose bpf_d_path helper to " KP Singh @ 2020-11-12 20:29 ` KP Singh 2 siblings, 0 replies; 6+ messages in thread From: KP Singh @ 2020-11-12 20:29 UTC (permalink / raw) To: open list, bpf On Thu, Nov 12, 2020 at 9:03 PM KP Singh <kpsingh@chromium.org> wrote: > > From: KP Singh <kpsingh@google.com> > > # v1 -> v2 > > * Fixed typos and formatting errors. > * Added Andrii's ack. Oops, I sent an older patch file which does not have Andrii's ack. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-12 23:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-12 20:03 [PATCH bpf-next v2 0/2] Sleepable LSM Hooks KP Singh 2020-11-12 20:03 ` [PATCH bpf-next v2 1/2] bpf: Augment the set of sleepable LSM hooks KP Singh 2020-11-12 22:35 ` Daniel Borkmann 2020-11-12 23:59 ` KP Singh 2020-11-12 20:03 ` [PATCH bpf-next v2 2/2] bpf: Expose bpf_d_path helper to " KP Singh 2020-11-12 20:29 ` [PATCH bpf-next v2 0/2] Sleepable LSM Hooks KP Singh
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).