bpf_link_free is always called in process context, including from a workqueue and from __fput. Neither of these have the ability to propagate an -ENOMEM to the caller. Reported-by: syzbot+83aa762ef23b6f0d1991@xxxxxxxxxxxxxxxxxxxxxxxxx Reported-by: syzbot+d29e58bb557324e55e5e@xxxxxxxxxxxxxxxxxxxxxxxxx Signed-off-by: Matt Mullins <mmullins@xxxxxxx> --- I previously referenced a "pretty ugly" patch. This is not that one, because I don't think there's any way I can make the caller of ->release() actually handle errors like ENOMEM. It also looks like most of the other ways tracepoint_probe_unregister is called also don't check the error code (e.g. just a quick grep found blk_unregister_tracepoints). Should this just be upgraded to GFP_NOFAIL across the board instead of passing around a gfp_t? include/linux/trace_events.h | 6 ++++-- include/linux/tracepoint.h | 7 +++++-- kernel/bpf/syscall.c | 2 +- kernel/trace/bpf_trace.c | 6 ++++-- kernel/trace/trace_events.c | 6 ++++-- kernel/tracepoint.c | 20 ++++++++++---------- 6 files changed, 28 insertions(+), 19 deletions(-) diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index 5c6943354049..166ad7646a98 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h @@ -625,7 +625,8 @@ int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog); void perf_event_detach_bpf_prog(struct perf_event *event); int perf_event_query_prog_array(struct perf_event *event, void __user *info); int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog); -int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog); +int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog, + gfp_t flags); struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name); void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp); int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, @@ -654,7 +655,8 @@ static inline int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_p { return -EOPNOTSUPP; } -static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *p) +static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, + struct bpf_prog *p, gfp_t flags) { return -EOPNOTSUPP; } diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h index 598fec9f9dbf..7b02f92f3b8f 100644 --- a/include/linux/tracepoint.h +++ b/include/linux/tracepoint.h @@ -12,6 +12,7 @@ * Heavily inspired from the Linux Kernel Markers. */ +#include <linux/gfp.h> #include <linux/smp.h> #include <linux/srcu.h> #include <linux/errno.h> @@ -40,7 +41,8 @@ extern int tracepoint_probe_register_prio(struct tracepoint *tp, void *probe, void *data, int prio); extern int -tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data); +tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data, + gfp_t flags); extern void for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv), void *priv); @@ -260,7 +262,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) unregister_trace_##name(void (*probe)(data_proto), void *data) \ { \ return tracepoint_probe_unregister(&__tracepoint_##name,\ - (void *)probe, data); \ + (void *)probe, data, \ + GFP_KERNEL); \ } \ static inline void \ check_trace_callback_type_##name(void (*cb)(data_proto)) \ diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index b999e7ff2583..f6876681c4ab 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -2601,7 +2601,7 @@ static void bpf_raw_tp_link_release(struct bpf_link *link) struct bpf_raw_tp_link *raw_tp = container_of(link, struct bpf_raw_tp_link, link); - bpf_probe_unregister(raw_tp->btp, raw_tp->link.prog); + bpf_probe_unregister(raw_tp->btp, raw_tp->link.prog, GFP_KERNEL | __GFP_NOFAIL); bpf_put_raw_tracepoint(raw_tp->btp); } diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index a8d4f253ed77..a4ea58c7506d 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1955,9 +1955,11 @@ int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog) return __bpf_probe_register(btp, prog); } -int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog) +int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog, + gfp_t flags) { - return tracepoint_probe_unregister(btp->tp, (void *)btp->bpf_func, prog); + return tracepoint_probe_unregister(btp->tp, (void *)btp->bpf_func, prog, + flags); } int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index a85effb2373b..ab1ac89caed2 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c @@ -296,7 +296,8 @@ int trace_event_reg(struct trace_event_call *call, case TRACE_REG_UNREGISTER: tracepoint_probe_unregister(call->tp, call->class->probe, - file); + file, + GFP_KERNEL); return 0; #ifdef CONFIG_PERF_EVENTS @@ -307,7 +308,8 @@ int trace_event_reg(struct trace_event_call *call, case TRACE_REG_PERF_UNREGISTER: tracepoint_probe_unregister(call->tp, call->class->perf_probe, - call); + call, + GFP_KERNEL); return 0; case TRACE_REG_PERF_OPEN: case TRACE_REG_PERF_CLOSE: diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c index 73956eaff8a9..619666a43c9f 100644 --- a/kernel/tracepoint.c +++ b/kernel/tracepoint.c @@ -53,10 +53,9 @@ struct tp_probes { struct tracepoint_func probes[0]; }; -static inline void *allocate_probes(int count) +static inline void *allocate_probes(int count, gfp_t flags) { - struct tp_probes *p = kmalloc(struct_size(p, probes, count), - GFP_KERNEL); + struct tp_probes *p = kmalloc(struct_size(p, probes, count), flags); return p == NULL ? NULL : p->probes; } @@ -150,7 +149,7 @@ func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func, } } /* + 2 : one for new probe, one for NULL func */ - new = allocate_probes(nr_probes + 2); + new = allocate_probes(nr_probes + 2, GFP_KERNEL); if (new == NULL) return ERR_PTR(-ENOMEM); if (old) { @@ -174,7 +173,7 @@ func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func, } static void *func_remove(struct tracepoint_func **funcs, - struct tracepoint_func *tp_func) + struct tracepoint_func *tp_func, gfp_t flags) { int nr_probes = 0, nr_del = 0, i; struct tracepoint_func *old, *new; @@ -207,7 +206,7 @@ static void *func_remove(struct tracepoint_func **funcs, int j = 0; /* N -> M, (N > 1, M > 0) */ /* + 1 for NULL */ - new = allocate_probes(nr_probes - nr_del + 1); + new = allocate_probes(nr_probes - nr_del + 1, flags); if (new == NULL) return ERR_PTR(-ENOMEM); for (i = 0; old[i].func; i++) @@ -264,13 +263,13 @@ static int tracepoint_add_func(struct tracepoint *tp, * by preempt_disable around the call site. */ static int tracepoint_remove_func(struct tracepoint *tp, - struct tracepoint_func *func) + struct tracepoint_func *func, gfp_t flags) { struct tracepoint_func *old, *tp_funcs; tp_funcs = rcu_dereference_protected(tp->funcs, lockdep_is_held(&tracepoints_mutex)); - old = func_remove(&tp_funcs, func); + old = func_remove(&tp_funcs, func, flags); if (IS_ERR(old)) { WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM); return PTR_ERR(old); @@ -344,7 +343,8 @@ EXPORT_SYMBOL_GPL(tracepoint_probe_register); * * Returns 0 if ok, error value on error. */ -int tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data) +int tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data, + gfp_t flags) { struct tracepoint_func tp_func; int ret; @@ -352,7 +352,7 @@ int tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data) mutex_lock(&tracepoints_mutex); tp_func.func = probe; tp_func.data = data; - ret = tracepoint_remove_func(tp, &tp_func); + ret = tracepoint_remove_func(tp, &tp_func, flags); mutex_unlock(&tracepoints_mutex); return ret; } -- 2.17.1