Re: [PATCH bpf-next v2] bpf: support raw tracepoints in modules
On Wed, Dec 12, 2018 at 04:42:37PM -0800, Matt Mullins wrote: > Distributions build drivers as modules, including network and filesystem > drivers which export numerous tracepoints. This enables > bpf(BPF_RAW_TRACEPOINT_OPEN) to attach to those tracepoints. > > Signed-off-by: Matt Mullins > --- > v1->v2: > * avoid taking the mutex in bpf_event_notify when op is neither COMING nor > GOING. > * check that kzalloc actually succeeded Applied, Thanks
Re: [PATCH bpf-next v2] bpf: support raw tracepoints in modules
On Thu, Dec 13, 2018 at 11:38:51AM -0800, Matt Mullins wrote: > On Thu, 2018-12-13 at 19:22 +, Martin Lau wrote: > > On Wed, Dec 12, 2018 at 04:42:37PM -0800, Matt Mullins wrote: > > > Distributions build drivers as modules, including network and filesystem > > > drivers which export numerous tracepoints. This enables > > > bpf(BPF_RAW_TRACEPOINT_OPEN) to attach to those tracepoints. > > > Acked-by: Martin KaFai Lau [ ... ] > > > +#ifdef CONFIG_MODULES > > > +int bpf_event_notify(struct notifier_block *nb, unsigned long op, void > > > *module) > > > +{ > > > + struct bpf_trace_module *btm, *tmp; > > > + struct module *mod = module; > > > + > > > + if (mod->num_bpf_raw_events == 0 || > > > + (op != MODULE_STATE_COMING && op != MODULE_STATE_GOING)) > > > + return 0; > > > + > > > + mutex_lock(&bpf_module_mutex); > > > + > > > + switch (op) { > > > + case MODULE_STATE_COMING: > > > + btm = kzalloc(sizeof(*btm), GFP_KERNEL); > > > + if (btm) { > > > + btm->module = module; > > > + list_add(&btm->list, &bpf_trace_modules); > > > + } > > > > Is it fine to return 0 on !btm case? > > That effectively just means we'll be ignoring tracepoints for a module > that is loaded while we can't allocate a bpf_trace_module (24 bytes) to > track it. That feels like reasonable behavior to me. ok.
Re: [PATCH bpf-next v2] bpf: support raw tracepoints in modules
On Thu, 2018-12-13 at 19:22 +, Martin Lau wrote: > On Wed, Dec 12, 2018 at 04:42:37PM -0800, Matt Mullins wrote: > > Distributions build drivers as modules, including network and filesystem > > drivers which export numerous tracepoints. This enables > > bpf(BPF_RAW_TRACEPOINT_OPEN) to attach to those tracepoints. > > > > Signed-off-by: Matt Mullins > > --- > > v1->v2: > > * avoid taking the mutex in bpf_event_notify when op is neither COMING nor > > GOING. > > * check that kzalloc actually succeeded > > > > I didn't try to check list_empty before taking the mutex since I want to > > avoid > > races between bpf_event_notify and bpf_get_raw_tracepoint. Additionally, > > list_for_each_entry_safe is not strictly necessary upon MODULE_STATE_GOING, > > but > > Alexei suggested I use it to protect against fragility if the subsequent > > break; > > eventually disappears. > > > > include/linux/module.h | 4 ++ > > include/linux/trace_events.h | 8 ++- > > kernel/bpf/syscall.c | 11 ++-- > > kernel/module.c | 5 ++ > > kernel/trace/bpf_trace.c | 99 +++- > > 5 files changed, 120 insertions(+), 7 deletions(-) > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > index fce6b4335e36..5f147dd5e709 100644 > > --- a/include/linux/module.h > > +++ b/include/linux/module.h > > @@ -432,6 +432,10 @@ struct module { > > unsigned int num_tracepoints; > > tracepoint_ptr_t *tracepoints_ptrs; > > #endif > > +#ifdef CONFIG_BPF_EVENTS > > + unsigned int num_bpf_raw_events; > > + struct bpf_raw_event_map *bpf_raw_events; > > +#endif > > #ifdef HAVE_JUMP_LABEL > > struct jump_entry *jump_entries; > > unsigned int num_jump_entries; > > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > > index 4130a5497d40..8a62731673f7 100644 > > --- a/include/linux/trace_events.h > > +++ b/include/linux/trace_events.h > > @@ -471,7 +471,8 @@ 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); > > -struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name); > > +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, > > u32 *fd_type, const char **buf, > > u64 *probe_offset, u64 *probe_addr); > > @@ -502,10 +503,13 @@ static inline int bpf_probe_unregister(struct > > bpf_raw_event_map *btp, struct bpf > > { > > return -EOPNOTSUPP; > > } > > -static inline struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char > > *name) > > +static inline struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char > > *name) > > { > > return NULL; > > } > > +static inline void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp) > > +{ > > +} > > static inline int bpf_get_perf_event_info(const struct perf_event *event, > > u32 *prog_id, u32 *fd_type, > > const char **buf, u64 *probe_offset, > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index 70fb11106fc2..754370e3155e 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -1609,6 +1609,7 @@ static int bpf_raw_tracepoint_release(struct inode > > *inode, struct file *filp) > > bpf_probe_unregister(raw_tp->btp, raw_tp->prog); > > bpf_prog_put(raw_tp->prog); > > } > > + bpf_put_raw_tracepoint(raw_tp->btp); > > kfree(raw_tp); > > return 0; > > } > > @@ -1634,13 +1635,15 @@ static int bpf_raw_tracepoint_open(const union > > bpf_attr *attr) > > return -EFAULT; > > tp_name[sizeof(tp_name) - 1] = 0; > > > > - btp = bpf_find_raw_tracepoint(tp_name); > > + btp = bpf_get_raw_tracepoint(tp_name); > > if (!btp) > > return -ENOENT; > > > > raw_tp = kzalloc(sizeof(*raw_tp), GFP_USER); > > - if (!raw_tp) > > - return -ENOMEM; > > + if (!raw_tp) { > > + err = -ENOMEM; > > + goto out_put_btp; > > + } > > raw_tp->btp = btp; > > > > prog = bpf_prog_get_type(attr->raw_tracepoint.prog_fd, > > @@ -1668,6 +1671,8 @@ static int bpf_raw_tracepoint_open(const union > > bpf_attr *attr) > > bpf_prog_put(prog); > > out_free_tp: > > kfree(raw_tp); > > +out_put_btp: > > + bpf_put_raw_tracepoint(btp); > > return err; > > } > > > > diff --git a/kernel/module.c b/kernel/module.c > > index 49a405891587..06ec68f08387 100644 > > --- a/kernel/module.c > > +++ b/kernel/module.c > > @@ -3093,6 +3093,11 @@ static int find_module_sections(struct
Re: [PATCH bpf-next v2] bpf: support raw tracepoints in modules
On Wed, Dec 12, 2018 at 04:42:37PM -0800, Matt Mullins wrote: > Distributions build drivers as modules, including network and filesystem > drivers which export numerous tracepoints. This enables > bpf(BPF_RAW_TRACEPOINT_OPEN) to attach to those tracepoints. > > Signed-off-by: Matt Mullins > --- > v1->v2: > * avoid taking the mutex in bpf_event_notify when op is neither COMING nor > GOING. > * check that kzalloc actually succeeded > > I didn't try to check list_empty before taking the mutex since I want to avoid > races between bpf_event_notify and bpf_get_raw_tracepoint. Additionally, > list_for_each_entry_safe is not strictly necessary upon MODULE_STATE_GOING, > but > Alexei suggested I use it to protect against fragility if the subsequent > break; > eventually disappears. > > include/linux/module.h | 4 ++ > include/linux/trace_events.h | 8 ++- > kernel/bpf/syscall.c | 11 ++-- > kernel/module.c | 5 ++ > kernel/trace/bpf_trace.c | 99 +++- > 5 files changed, 120 insertions(+), 7 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index fce6b4335e36..5f147dd5e709 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -432,6 +432,10 @@ struct module { > unsigned int num_tracepoints; > tracepoint_ptr_t *tracepoints_ptrs; > #endif > +#ifdef CONFIG_BPF_EVENTS > + unsigned int num_bpf_raw_events; > + struct bpf_raw_event_map *bpf_raw_events; > +#endif > #ifdef HAVE_JUMP_LABEL > struct jump_entry *jump_entries; > unsigned int num_jump_entries; > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > index 4130a5497d40..8a62731673f7 100644 > --- a/include/linux/trace_events.h > +++ b/include/linux/trace_events.h > @@ -471,7 +471,8 @@ 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); > -struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name); > +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, > u32 *fd_type, const char **buf, > u64 *probe_offset, u64 *probe_addr); > @@ -502,10 +503,13 @@ static inline int bpf_probe_unregister(struct > bpf_raw_event_map *btp, struct bpf > { > return -EOPNOTSUPP; > } > -static inline struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char > *name) > +static inline struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char > *name) > { > return NULL; > } > +static inline void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp) > +{ > +} > static inline int bpf_get_perf_event_info(const struct perf_event *event, > u32 *prog_id, u32 *fd_type, > const char **buf, u64 *probe_offset, > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 70fb11106fc2..754370e3155e 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -1609,6 +1609,7 @@ static int bpf_raw_tracepoint_release(struct inode > *inode, struct file *filp) > bpf_probe_unregister(raw_tp->btp, raw_tp->prog); > bpf_prog_put(raw_tp->prog); > } > + bpf_put_raw_tracepoint(raw_tp->btp); > kfree(raw_tp); > return 0; > } > @@ -1634,13 +1635,15 @@ static int bpf_raw_tracepoint_open(const union > bpf_attr *attr) > return -EFAULT; > tp_name[sizeof(tp_name) - 1] = 0; > > - btp = bpf_find_raw_tracepoint(tp_name); > + btp = bpf_get_raw_tracepoint(tp_name); > if (!btp) > return -ENOENT; > > raw_tp = kzalloc(sizeof(*raw_tp), GFP_USER); > - if (!raw_tp) > - return -ENOMEM; > + if (!raw_tp) { > + err = -ENOMEM; > + goto out_put_btp; > + } > raw_tp->btp = btp; > > prog = bpf_prog_get_type(attr->raw_tracepoint.prog_fd, > @@ -1668,6 +1671,8 @@ static int bpf_raw_tracepoint_open(const union bpf_attr > *attr) > bpf_prog_put(prog); > out_free_tp: > kfree(raw_tp); > +out_put_btp: > + bpf_put_raw_tracepoint(btp); > return err; > } > > diff --git a/kernel/module.c b/kernel/module.c > index 49a405891587..06ec68f08387 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -3093,6 +3093,11 @@ static int find_module_sections(struct module *mod, > struct load_info *info) >sizeof(*mod->tracepoints_ptrs), >&mod->num_tracepoints); > #endif > +#ifdef CONFIG_BPF_EVENTS > + mod->bpf_raw
[PATCH bpf-next v2] bpf: support raw tracepoints in modules
Distributions build drivers as modules, including network and filesystem drivers which export numerous tracepoints. This enables bpf(BPF_RAW_TRACEPOINT_OPEN) to attach to those tracepoints. Signed-off-by: Matt Mullins --- v1->v2: * avoid taking the mutex in bpf_event_notify when op is neither COMING nor GOING. * check that kzalloc actually succeeded I didn't try to check list_empty before taking the mutex since I want to avoid races between bpf_event_notify and bpf_get_raw_tracepoint. Additionally, list_for_each_entry_safe is not strictly necessary upon MODULE_STATE_GOING, but Alexei suggested I use it to protect against fragility if the subsequent break; eventually disappears. include/linux/module.h | 4 ++ include/linux/trace_events.h | 8 ++- kernel/bpf/syscall.c | 11 ++-- kernel/module.c | 5 ++ kernel/trace/bpf_trace.c | 99 +++- 5 files changed, 120 insertions(+), 7 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index fce6b4335e36..5f147dd5e709 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -432,6 +432,10 @@ struct module { unsigned int num_tracepoints; tracepoint_ptr_t *tracepoints_ptrs; #endif +#ifdef CONFIG_BPF_EVENTS + unsigned int num_bpf_raw_events; + struct bpf_raw_event_map *bpf_raw_events; +#endif #ifdef HAVE_JUMP_LABEL struct jump_entry *jump_entries; unsigned int num_jump_entries; diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index 4130a5497d40..8a62731673f7 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h @@ -471,7 +471,8 @@ 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); -struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name); +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, u32 *fd_type, const char **buf, u64 *probe_offset, u64 *probe_addr); @@ -502,10 +503,13 @@ static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf { return -EOPNOTSUPP; } -static inline struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name) +static inline struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name) { return NULL; } +static inline void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp) +{ +} static inline int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, u32 *fd_type, const char **buf, u64 *probe_offset, diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 70fb11106fc2..754370e3155e 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1609,6 +1609,7 @@ static int bpf_raw_tracepoint_release(struct inode *inode, struct file *filp) bpf_probe_unregister(raw_tp->btp, raw_tp->prog); bpf_prog_put(raw_tp->prog); } + bpf_put_raw_tracepoint(raw_tp->btp); kfree(raw_tp); return 0; } @@ -1634,13 +1635,15 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr) return -EFAULT; tp_name[sizeof(tp_name) - 1] = 0; - btp = bpf_find_raw_tracepoint(tp_name); + btp = bpf_get_raw_tracepoint(tp_name); if (!btp) return -ENOENT; raw_tp = kzalloc(sizeof(*raw_tp), GFP_USER); - if (!raw_tp) - return -ENOMEM; + if (!raw_tp) { + err = -ENOMEM; + goto out_put_btp; + } raw_tp->btp = btp; prog = bpf_prog_get_type(attr->raw_tracepoint.prog_fd, @@ -1668,6 +1671,8 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr) bpf_prog_put(prog); out_free_tp: kfree(raw_tp); +out_put_btp: + bpf_put_raw_tracepoint(btp); return err; } diff --git a/kernel/module.c b/kernel/module.c index 49a405891587..06ec68f08387 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -3093,6 +3093,11 @@ static int find_module_sections(struct module *mod, struct load_info *info) sizeof(*mod->tracepoints_ptrs), &mod->num_tracepoints); #endif +#ifdef CONFIG_BPF_EVENTS + mod->bpf_raw_events = section_objs(info, "__bpf_raw_tp_map", + sizeof(*mod->bpf_raw_events), + &mod->num_bpf_raw_events); +#endif #ifdef HAVE_JUMP_LABEL mod->jump