[PATCH] bpf: Enforce map preallocation for all instrumentation programs
From: Thomas Gleixner <hidden>
Date: 2020-02-17 12:59:42
Also in:
bpf, lkml
The assumption that only programs attached to perf NMI events can deadlock
on memory allocators is wrong. Assume the following simplified callchain:
kmalloc() from regular non BPF context
cache empty
freelist empty
lock(zone->lock);
tracepoint or kprobe
BPF()
update_elem()
lock(bucket)
kmalloc()
cache empty
freelist empty
lock(zone->lock); <- DEADLOCK
There are also other ways to create wreckage:
kmalloc() from regular non BPF context
local_irq_save();
...
obj = slab_first();
kprobe()
BPF()
update_elem()
lock(bucket)
kmalloc()
local_irq_save();
...
obj = slab_first(); <- Same object as above ...
So preallocation _must_ be enforced for all variants of intrusive
instrumentation.
Signed-off-by: Thomas Gleixner <redacted>
---
kernel/bpf/verifier.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c@@ -8144,19 +8144,23 @@ static int check_map_prog_compatibility( struct bpf_prog *prog) { - /* Make sure that BPF_PROG_TYPE_PERF_EVENT programs only use - * preallocated hash maps, since doing memory allocation - * in overflow_handler can crash depending on where nmi got - * triggered. + /* + * Make sure that trace type programs use preallocated hash maps. + * Perf programs obviously can't do memory allocation in NMI + * context and all other types can deadlock on a memory allocator + * lock when a tracepoint/kprobe triggers a BPF program inside a + * lock held region or create inconsistent state when the probe is + * within an interrupts disabled critical region in the memory + * allocator. */ - if (prog->type == BPF_PROG_TYPE_PERF_EVENT) { + if ((is_tracing_prog_type(prog->type)) { if (!check_map_prealloc(map)) { - verbose(env, "perf_event programs can only use preallocated hash map\n"); + verbose(env, "tracing programs can only use preallocated hash map\n"); return -EINVAL; } if (map->inner_map_meta && !check_map_prealloc(map->inner_map_meta)) { - verbose(env, "perf_event programs can only use preallocated inner hash map\n"); + verbose(env, "tracing programs can only use preallocated inner hash map\n"); return -EINVAL; } }