| Message ID | 20240924152936.233013-2-massimiliano.pellizzer@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | CVE-2023-52621 | expand |
On 24/09/2024 17:29, Massimiliano Pellizzer wrote: > From: Hou Tao <houtao1@huawei.com> > > [ Upstream commit 169410eba271afc9f0fb476d996795aa26770c6d ] > > These three bpf_map_{lookup,update,delete}_elem() helpers are also > available for sleepable bpf program, so add the corresponding lock > assertion for sleepable bpf program, otherwise the following warning > will be reported when a sleepable bpf program manipulates bpf map under > interpreter mode (aka bpf_jit_enable=0): > > WARNING: CPU: 3 PID: 4985 at kernel/bpf/helpers.c:40 ...... > CPU: 3 PID: 4985 Comm: test_progs Not tainted 6.6.0+ #2 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ...... > RIP: 0010:bpf_map_lookup_elem+0x54/0x60 > ...... > Call Trace: > <TASK> > ? __warn+0xa5/0x240 > ? bpf_map_lookup_elem+0x54/0x60 > ? report_bug+0x1ba/0x1f0 > ? handle_bug+0x40/0x80 > ? exc_invalid_op+0x18/0x50 > ? asm_exc_invalid_op+0x1b/0x20 > ? __pfx_bpf_map_lookup_elem+0x10/0x10 > ? rcu_lockdep_current_cpu_online+0x65/0xb0 > ? rcu_is_watching+0x23/0x50 > ? bpf_map_lookup_elem+0x54/0x60 > ? __pfx_bpf_map_lookup_elem+0x10/0x10 > ___bpf_prog_run+0x513/0x3b70 > __bpf_prog_run32+0x9d/0xd0 > ? __bpf_prog_enter_sleepable_recur+0xad/0x120 > ? __bpf_prog_enter_sleepable_recur+0x3e/0x120 > bpf_trampoline_6442580665+0x4d/0x1000 > __x64_sys_getpgid+0x5/0x30 > ? do_syscall_64+0x36/0xb0 > entry_SYSCALL_64_after_hwframe+0x6e/0x76 > </TASK> > > Signed-off-by: Hou Tao <houtao1@huawei.com> > Link: https://lore.kernel.org/r/20231204140425.1480317-2-houtao@huaweicloud.com > Signed-off-by: Alexei Starovoitov <ast@kernel.org> > Signed-off-by: Sasha Levin <sashal@kernel.org> > (backported from commit d6d6fe4bb105595118f12abeed4a7bdd450853f3 linux-6.1.y) > [mpellizzer: added the missing header rcupdate_trace.h] > CVE-2023-52621 > Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> > --- > kernel/bpf/helpers.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index 273f2f0deb23..026f3a5e71a6 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -15,6 +15,7 @@ > #include <linux/pid_namespace.h> > #include <linux/proc_ns.h> > #include <linux/security.h> > +#include <linux/rcupdate_trace.h> > > #include "../../lib/kstrtox.h" > > @@ -24,12 +25,13 @@ > * > * Different map implementations will rely on rcu in map methods > * lookup/update/delete, therefore eBPF programs must run under rcu lock > - * if program is allowed to access maps, so check rcu_read_lock_held in > - * all three functions. > + * if program is allowed to access maps, so check rcu_read_lock_held() or > + * rcu_read_lock_trace_held() in all three functions. > */ > BPF_CALL_2(bpf_map_lookup_elem, struct bpf_map *, map, void *, key) > { > - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); > + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && > + !rcu_read_lock_bh_held()); > return (unsigned long) map->ops->map_lookup_elem(map, key); > } > > @@ -45,7 +47,8 @@ const struct bpf_func_proto bpf_map_lookup_elem_proto = { > BPF_CALL_4(bpf_map_update_elem, struct bpf_map *, map, void *, key, > void *, value, u64, flags) > { > - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); > + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && > + !rcu_read_lock_bh_held()); > return map->ops->map_update_elem(map, key, value, flags); > } > > @@ -62,7 +65,8 @@ const struct bpf_func_proto bpf_map_update_elem_proto = { > > BPF_CALL_2(bpf_map_delete_elem, struct bpf_map *, map, void *, key) > { > - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); > + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && > + !rcu_read_lock_bh_held()); > return map->ops->map_delete_elem(map, key); > } > Acked-by: Roxana Nicolescu <roxana.nicolescu@canonical.com>
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 273f2f0deb23..026f3a5e71a6 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -15,6 +15,7 @@ #include <linux/pid_namespace.h> #include <linux/proc_ns.h> #include <linux/security.h> +#include <linux/rcupdate_trace.h> #include "../../lib/kstrtox.h" @@ -24,12 +25,13 @@ * * Different map implementations will rely on rcu in map methods * lookup/update/delete, therefore eBPF programs must run under rcu lock - * if program is allowed to access maps, so check rcu_read_lock_held in - * all three functions. + * if program is allowed to access maps, so check rcu_read_lock_held() or + * rcu_read_lock_trace_held() in all three functions. */ BPF_CALL_2(bpf_map_lookup_elem, struct bpf_map *, map, void *, key) { - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); return (unsigned long) map->ops->map_lookup_elem(map, key); } @@ -45,7 +47,8 @@ const struct bpf_func_proto bpf_map_lookup_elem_proto = { BPF_CALL_4(bpf_map_update_elem, struct bpf_map *, map, void *, key, void *, value, u64, flags) { - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); return map->ops->map_update_elem(map, key, value, flags); } @@ -62,7 +65,8 @@ const struct bpf_func_proto bpf_map_update_elem_proto = { BPF_CALL_2(bpf_map_delete_elem, struct bpf_map *, map, void *, key) { - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); return map->ops->map_delete_elem(map, key); }