bpf_lsm_find_cgroup_shim() picks the shim that runs a BPF_LSM_CGROUP program from the first argument of the attached hook. It compares args[0].type, the BTF id of the argument's pointer type, against btf_sock_ids[], which holds the struct ids of struct socket and struct sock. A pointer id is never equal to a struct id, so neither branch is taken and every cgroup LSM program runs through __cgroup_bpf_run_lsm_current(). Socket and sock hooks therefore run against task_dfl_cgroup(current) instead of the socket's cgroup. When a socket is used from a cgroup other than the one that created it, the caller's cgroup policy is applied instead of the socket's, and __cgroup_bpf_run_lsm_socket() and __cgroup_bpf_run_lsm_sock() are dead code. Resolve the argument's pointee with btf_type_resolve_ptr() and compare that id against btf_sock_ids[]. This makes __cgroup_bpf_run_lsm_socket() reachable for calls it cannot handle: ip_cmsg_recv_security() passes a NULL socket to socket_getpeersec_dgram, and sock_create_lite() calls socket_post_create before sock->sk is set. Fall back to __cgroup_bpf_run_lsm_current() in those cases instead of dereferencing sock->sk. __cgroup_bpf_run_lsm_sock() needs no such fallback. sk_alloc_security and sk_free_security run while the sk has no cgroup and stay on bpf_lsm_current_hooks; every other hook whose first argument is a struct sock is called with a non-NULL sk whose cgroup is already set. Existing BPF_LSM_CGROUP users will see a behavior change. Socket and sock hooks now run the programs attached to the socket's cgroup, as described in commit 69fd337a975c ("bpf: per-cgroup lsm flavor"). Hooks that can run in softirq, such as socket_sock_rcv_skb, inet_conn_request and sk_getsecid, no longer depend on which task was interrupted. The bug was found with the help of an LLM. Fixes: 69fd337a975c ("bpf: per-cgroup lsm flavor") Cc: stable@vger.kernel.org Signed-off-by: Andrea Parri --- This changes behavior for existing BPF_LSM_CGROUP users: since 69fd337a975c, socket and sock hooks have run the programs of the current task's cgroup, and with this patch they run those of the socket's cgroup, as that commit intended. I'm sending it to bpf with Cc: stable because a socket used from another cgroup is currently checked against that cgroup's policy instead of its own. Happy to retarget bpf-next and drop the stable Cc if you prefer. kernel/bpf/bpf_lsm.c | 8 ++++++-- kernel/bpf/cgroup.c | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c index f58dce888ff4a..9547d4f351103 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -95,6 +95,7 @@ void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func) { const struct btf_param *args __maybe_unused; + u32 type_id __maybe_unused = 0; if (btf_type_vlen(prog->aux->attach_func_proto) < 1 || btf_id_set_contains(&bpf_lsm_current_hooks, @@ -106,9 +107,12 @@ void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, #ifdef CONFIG_NET args = btf_params(prog->aux->attach_func_proto); - if (args[0].type == btf_sock_ids[BTF_SOCK_TYPE_SOCKET]) + /* args[0] is a pointer; match the struct it points to. */ + btf_type_resolve_ptr(prog->aux->attach_btf, args[0].type, &type_id); + + if (type_id == btf_sock_ids[BTF_SOCK_TYPE_SOCKET]) *bpf_func = __cgroup_bpf_run_lsm_socket; - else if (args[0].type == btf_sock_ids[BTF_SOCK_TYPE_SOCK]) + else if (type_id == btf_sock_ids[BTF_SOCK_TYPE_SOCK]) *bpf_func = __cgroup_bpf_run_lsm_sock; else #endif diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index 57a785241d42f..9100d1c4f35ff 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -171,15 +171,20 @@ unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx, const struct bpf_prog *shim_prog; struct socket *sock; struct cgroup *cgrp; + struct sock *sk; int ret = 0; u64 *args; args = (u64 *)ctx; sock = (void *)(unsigned long)args[0]; + sk = sock ? sock->sk : NULL; + /* NULL from ip_cmsg_recv_security(), no sk yet in sock_create_lite() */ + if (unlikely(!sk)) + return __cgroup_bpf_run_lsm_current(ctx, insn); /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/ shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi)); - cgrp = sock_cgroup_ptr(&sock->sk->sk_cgrp_data); + cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data); if (likely(cgrp)) ret = bpf_prog_run_array_cg(&cgrp->bpf, shim_prog->aux->cgroup_atype, -- 2.53.0