On Thu, Aug 8, 2024 at 3:46 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote: > > On Tue, Aug 06, 2024 at 11:44:52AM -0700, Alexei Starovoitov wrote: > > On Tue, Aug 6, 2024 at 6:24 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote: > > > > > > > Jiri, > > > > > > > > the verifier removes the check because it assumes that pointers > > > > passed by the kernel into tracepoint are valid and trusted. > > > > In this case: > > > > trace_sched_pi_setprio(p, pi_task); > > > > > > > > pi_task can be NULL. > > > > > > > > We cannot make all tracepoint pointers to be PTR_TRUSTED | PTR_MAYBE_NULL > > > > by default, since it will break a bunch of progs. > > > > Instead we can annotate this tracepoint arg as __nullable and > > > > teach the verifier to recognize such special arguments of tracepoints. > > > > > > ok, so you mean to be able to mark it in event header like: > > > > > > TRACE_EVENT(sched_pi_setprio, > > > TP_PROTO(struct task_struct *tsk, struct task_struct *pi_task __nullable), > > > > > > I guess we could make pahole to emit DECL_TAG for that argument, > > > but I'm not sure how to propagate that __nullable info to pahole > > > > > > while wondering about that, I tried the direct fix below ;-) > > > > We don't need to rush such a hack below. > > No need to add decl_tag and change pahole either. > > The arg name is already vmlinux BTF: > > [51371] FUNC_PROTO '(anon)' ret_type_id=0 vlen=3 > > '__data' type_id=61 > > 'tsk' type_id=77 > > 'pi_task' type_id=77 > > [51372] FUNC '__bpf_trace_sched_pi_setprio' type_id=51371 linkage=static > > > > just need to rename "pi_task" to "pi_task__nullable" > > and teach the verifier. > > the problem is that btf_trace_<xxx> is typedef > > typedef void (*btf_trace_##call)(void *__data, proto); > > and dwarf does not store argument names for subroutine type entry, > so it's not in BTF's TYPEDEF either > > it's the btf_trace_##call typedef ID that verifier has to work with, > I wonder we could somehow associate that ID with __bpf_trace_##call > subroutine entry which has the argument names > > we could store __bpf_trace_##call's BTF_ID in __bpf_raw_tp_map record, > but we'd need to do the lookup based on the tracepoint name when loading > the program .. ATM we do the lookup __bpf_raw_tp_map record only when > doing attach, so we would need to move it to program load time > > or we could 'fix' the argument names in pahole, but that'd probably > mean extra setup and hash lookup, so also not great I would do a simple string search in vmlinux BTF for "__bpf_trace" + tp name. No need to add btf_id-s and waste memory to speed up the slow path.
