On Mon, 19 Aug 2024 13:47:20 +0200
Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
> verifier assumes that programs attached to the tracepoint can access
> pointer arguments without checking them for null and some of those
> programs most likely access such arguments directly
Hmm, so the verifier made a wrong assumption :-/ That's because that was
never a requirement for tracepoint arguments and several can easily be
NULL. That's why the macros have NULL checks for all arguments. For
example, see include/trace/stages/stage5_get_offsets.h:
static inline const char *__string_src(const char *str)
{
if (!str)
return EVENT_NULL_STR;
return str;
}
How does the verifier handle accessing function arguments? Because a
tracepoint call is no different.
>
> changing that globally and require bpf program to do null check for all
> pointer arguments will make verifier fail to load existing programs
>
> >
> > If you had a macro around the parameter:
> >
> > TP_PROTO(struct task_struct *tsk, struct task_struct *__nullable(pi_task)),
> >
> > Could having that go through another macro pass in trace_events.h work?
> > That is, could we associate the trace event with "nullable" parameters
> > that could be stored someplace else for you?
>
> IIUC you mean to store extra data for each tracepoint that would
> annotate the argument? as Alexei pointed out earlier it might be
> too much, because we'd be fine with just adding suffix to annotated
> arguments in __bpf_trace_##call:
>
> __bpf_trace_##call(void *__data, proto) \
> { \
> CONCATENATE(bpf_trace_run, COUNT_ARGS(args))(__data, CAST_TO_U64(args)); \
> }
>
> with that verifier could easily get suffix information from BTF and
> once gcc implements btf_type_tag we can easily switch to that
Could it be possible that the verifier could add to the exception table for
all accesses to tracepoint arguments? Then if there's a NULL pointer
dereference, the kernel will not crash but the exception can be sent to the
user space process instead? That is, it sends SIGSEV to the task accessing
NULL when it shouldn't.
-- Steve
