[patch V2 01/20] bpf: Enforce preallocation for all instrumentation programs

From: Thomas Gleixner
Date: Thu Feb 20 2020 - 15:58:28 EST


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 other ways which do not involve locking to create wreckage:

kmalloc() from regular non BPF context
local_irq_save();
...
obj = percpu_slab_first();
kprobe()
BPF()
update_elem()
lock(bucket)
kmalloc()
local_irq_save();
...
obj = percpu_slab_first(); <- Same object as above ...

So preallocation _must_ be enforced for all variants of intrusive
instrumentation.

Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
---
V2: New patch
---
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 only 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;
}
}