On Mon, Jun 29, 2020 at 2:59 AM Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote: > > Both sockmap and flow_dissector ingnore various arguments passed to > BPF_PROG_ATTACH and BPF_PROG_DETACH. We can fix the attach case by > checking that the unused arguments are zero. I considered requiring > target_fd to be -1 instead of 0, but this leads to a lot of churn > in selftests. There is also precedent in that bpf_iter already > expects 0 for a similar field. I think that we can come up with a > work around for fd 0 should we need to in the future. > > The detach case is more problematic: both cgroups and lirc2 verify > that attach_bpf_fd matches the currently attached program. This > way you need access to the program fd to be able to remove it. > Neither sockmap nor flow_dissector do this. flow_dissector even > has a check for CAP_NET_ADMIN because of this. The patch set > addresses this by implementing the desired behaviour. > > There is a possibility for user space breakage: any callers that > don't provide the correct fd will fail with ENOENT. For sockmap > the risk is low: even the selftests assume that sockmap works > the way I described. For flow_dissector the story is less > straightforward, and the selftests use a variety of arguments. > > I've includes fixes tags for the oldest commits that allow an easy > backport, however the behaviour dates back to when sockmap and > flow_dissector were introduced. What is the best way to handle these? > > This set is based on top of Jakub's work "bpf, netns: Prepare > for multi-prog attachment" available at > https://lore.kernel.org/bpf/87k0zwmhtb.fsf@xxxxxxxxxxxxxx/T/ Folks, you should have used 'bpf' in the subject of Jakub's patches. I've dropped Jakub's set from bpf-next and re-applied to bpf tree. And applied this set on top. Thanks!
