On Mon, Jul 11, 2022 at 1:51 AM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 7/9/22 8:44 AM, Yafang Shao wrote:
> > The raw tracepoint may cause unexpected memory allocation if we set
> > BPF_F_NO_PREALLOC. So let's warn on it.
>
> Please extend raw_tracepoint to other attach types which
> may cause runtime map allocations.
>
Per my understanding, it is safe to allocate memory in a non-process
context as long as we don't allow blocking it.
So this issue doesn't matter with whether it causes runtime map
allocations or not, while it really matters with the tracepoint or
kprobe.
Regarding the tracepoint or kprobe, if we don't use non-preallocated
maps, it may allocate other extra memory besides the map element
itself.
I have verified that it is safe to use non-preallocated maps in
BPF_TRACE_ITER or BPF_TRACE_FENTRY.
So filtering out BPF_TRACE_RAW_TP only is enough.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > ---
> > kernel/bpf/verifier.c | 18 +++++++++++++-----
> > 1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index e3cf6194c24f..3cd8260827e0 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -12574,14 +12574,20 @@ static int check_map_prealloc(struct bpf_map *map)
> > !(map->map_flags & BPF_F_NO_PREALLOC);
> > }
> >
> > -static bool is_tracing_prog_type(enum bpf_prog_type type)
> > +static bool is_tracing_prog_type(enum bpf_prog_type prog_type,
> > + enum bpf_attach_type attach_type)
> > {
> > - switch (type) {
> > + switch (prog_type) {
> > case BPF_PROG_TYPE_KPROBE:
> > case BPF_PROG_TYPE_TRACEPOINT:
> > case BPF_PROG_TYPE_PERF_EVENT:
> > case BPF_PROG_TYPE_RAW_TRACEPOINT:
> > + case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
> > return true;
> > + case BPF_PROG_TYPE_TRACING:
> > + if (attach_type == BPF_TRACE_RAW_TP)
> > + return true;
>
> As Alexei mentioned earlier, here we should have
> if (attach_type != BPF_TRACE_ITER)
> return true;
That will break selftests/bpf/progs/timer.c, because it uses timer in fentry.
> For attach types with BPF_PROG_TYPE_TRACING programs,
> BPF_TRACE_ITER attach type can only appear in process context.
> All other attach types may appear in non-process context.
>
Thanks for the explanation.
> > + return false;
> > default:
> > return false;
> > }
> > @@ -12601,7 +12607,9 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
> > struct bpf_prog *prog)
> >
> > {
> > + enum bpf_attach_type attach_type = prog->expected_attach_type;
> > enum bpf_prog_type prog_type = resolve_prog_type(prog);
> > +
> [...]
--
Regards
Yafang
