On Wed, Jul 5, 2023 at 4:19 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> On 6/28/23 1:53 PM, Yafang Shao wrote:
> > To avoid returning uninitialized or random values when querying the file
> > descriptor (fd) and accessing probe_addr, it is necessary to clear the
> > variable prior to its use.
> >
> > Fixes: 41bdc4b40ed6 ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > Acked-by: Yonghong Song <yhs@xxxxxx>
> > ---
> > kernel/trace/bpf_trace.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 1f9f78e1992f..ac9958907a7c 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2382,10 +2382,12 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
> > event->attr.type == PERF_TYPE_TRACEPOINT);
> > #endif
> > #ifdef CONFIG_UPROBE_EVENTS
> > - if (flags & TRACE_EVENT_FL_UPROBE)
> > + if (flags & TRACE_EVENT_FL_UPROBE) {
> > err = bpf_get_uprobe_info(event, fd_type, buf,
> > probe_offset,
> > event->attr.type == PERF_TYPE_TRACEPOINT);
> > + *probe_addr = 0x0;
> > + }
>
> Could we make this a bit more robust by just moving the zero'ing into the common path?
Agree. Will change it.
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 03b7f6b8e4f0..795e16d5d2f7 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2362,6 +2362,9 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
> return -EOPNOTSUPP;
>
> *prog_id = prog->aux->id;
> + *probe_offset = 0x0;
> + *probe_addr = 0x0;
> +
> flags = event->tp_event->flags;
> is_tracepoint = flags & TRACE_EVENT_FL_TRACEPOINT;
> is_syscall_tp = is_syscall_trace_event(event->tp_event);
> @@ -2370,8 +2373,6 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
> *buf = is_tracepoint ? event->tp_event->tp->name
> : event->tp_event->name;
> *fd_type = BPF_FD_TYPE_TRACEPOINT;
> - *probe_offset = 0x0;
> - *probe_addr = 0x0;
> } else {
> /* kprobe/uprobe */
> err = -EOPNOTSUPP;
--
Regards
Yafang
