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. All this will preserve the lazy approach we have with no need to look ahead into subprog's implementation. I'd keep the concept of global subprog completely and exhaustively described with its type signature and associated tags as much as possible. P.S. We still need to keep in mind freplace complications, of course.
