On Fri, Sep 8, 2023 at 6:49 AM Alan Maguire alan.maguire@oracle.com wrote:

On 07/09/2023 21:06, Jiri Olsa wrote:

...

Currently the multi_kprobe link attach does not check error injection list for programs with bpf_override_return helper and allows them to attach anywhere. Adding the missing check.

Cc: stable@vger.kernel.org Fixes: 0dcac2725406 ("bpf: Add multi kprobe link") Signed-off-by: Jiri Olsa jolsa@kernel.org

For the series,

Reviewed-by: Alan Maguire alan.maguire@oracle.com

...with one small question below...

...

---

kernel/trace/bpf_trace.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index a7264b2c17ad..c1c1af63ced2 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2853,6 +2853,17 @@ static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u3 return arr.mods_cnt; }

+static int addrs_check_error_injection_list(unsigned long *addrs, u32 cnt) +{

• u32 i;
  

• for (i = 0; i < cnt; i++) {
  

•         if (!within_error_injection_list(addrs[i]))
  

•                 return -EINVAL;
  

do we need a check like trace_kprobe_on_func_entry() to verify that it's a combination of function entry+kprobe override, or is that handled elsewhere/not needed? perf_event_attach_bpf_prog() does

multi-kprobe programs are always attached at function entry, so I believe it's not necessary?

/*

• Kprobe override only works if they are on the function entry,
• and only if they are on the opt-in list.

*/ if (prog->kprobe_override && (!trace_kprobe_on_func_entry(event->tp_event) || !trace_kprobe_error_injectable(event->tp_event))) return -EINVAL;

if this is needed, it might be good to augment the selftest to cover the case of specifying non-entry+kprobe override. thanks!

Alan

...

• return 0;
  

+}

int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) { struct bpf_kprobe_multi_link *link = NULL; @@ -2930,6 +2941,11 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr goto error; }

• if (prog->kprobe_override && addrs_check_error_injection_list(addrs, cnt)) {
  

•         err = -EINVAL;
  

•         goto error;
  

• }
  

• link = kzalloc(sizeof(*link), GFP_KERNEL);
  if (!link) {
          err = -ENOMEM;