On Thu, Dec 5, 2024 at 10:23 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Thu, Dec 5, 2024 at 8:07 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > > > On Thu, 2024-12-05 at 17:44 -0800, Alexei Starovoitov wrote: > > > On Thu, Dec 5, 2024 at 4:29 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > > > > > > > so I went ahead and the fix does look simple: > > > > https://github.com/eddyz87/bpf/tree/skb-pull-data-global-func-bug > > > > > > Looks simple enough to me. > > > Ship it for bpf tree. > > > If we can come up with something better we can do it later in bpf-next. > > > > > > I very much prefer to avoid complexity as much as possible. > > > > Sent the patch-set for "simple". > > It is better then "dumb" by any metric anyways. > > Will try what Andrii suggests, as allowing calling global sub-programs > > from non-sleepable context sounds interesting. > > > > I haven't looked at your patches yet, but keep in mind another gotcha > with subprograms: they can be freplace'd by another BPF program > (clearly freplace programs were a successful reduction of > complexity... ;) > > What this means in practice is whatever deductions you get out of > analyzing any specific original subprogram might be violated by > freplace program if we don't enforce them during freplace attachment. > > > Anyways, I came here to say that I think I have a much simpler > solution that won't require big changes to the BPF verifier: tags. We > can shift the burden to the user having to declare the intent upfront > through subprog tags. And then, during verification of that global > subprog, the verifier can enforce that only explicitly declared side > effects can be enacted by the subprogram's code (taking into account > lazy dead code detection logic). > > We already take advantage of declarative tags for global subprog args > (__arg_trusted, etc), we can do the same for the function itself. We > can have __subprog_invalidates_all_pkt_pointers tag (and yes, I do > insist on this laconic name, of course), and during verification of > subprogram we just make sure that subprog was annotated as such, if > one of those fancy helpers is called directly in subprog itself or > transitively through any of *actually* called subprogs. tags for args was an aid to the verifier. Nothing is broken without them. Here it's about correctness. So we cannot use tags to solve this case.
