netdev - Re: bpf: ability to attach freplace to multiple parents

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ftdldkvl.fsf@toke.dk>
Date:   Fri, 03 Apr 2020 10:38:38 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andriin@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Andrey Ignatov <rdna@...com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: bpf: ability to attach freplace to multiple parents

Alexei Starovoitov <alexei.starovoitov@...il.com> writes:

> It's a different link.
> For fentry/fexit/freplace the link is pair:
>   // target           ...         bpf_prog
> (target_prog_fd_or_vmlinux, fentry_exit_replace_prog_fd).
>
> So for xdp case we will have:
> root_link = (eth0_ifindex, dispatcher_prog_fd) // dispatcher prog attached to eth0
> link1 = (dispatcher_prog_fd, xdp_firewall1_fd) // 1st extension prog attached to dispatcher
> link2 = (dispatcher_prog_fd, xdp_firewall2_fd) // 2nd extension prog attached to dispatcher
>
> Now libxdp wants to update the dispatcher prog.
> It generates new dispatcher prog with more placeholder entries or new policy:
> new_dispatcher_prog_fd.
> It's not attached anywhere.
> Then libxdp calls new bpf_raw_tp_open() api I'm proposing above to create:
> link3 = (new_dispatcher_prog_fd, xdp_firewall1_fd)
> link4 = (new_dispatcher_prog_fd, xdp_firewall2_fd)
> Now we have two firewalls attached to both old dispatcher prog and new dispatcher prog.
> Both firewalls are executing via old dispatcher prog that is active.
> Now libxdp calls:
> bpf_link_udpate(root_link, dispatcher_prog_fd, new_dispatcher_prog_fd)
> which atomically replaces old dispatcher prog with new dispatcher prog in eth0.
> The traffic keeps flowing into both firewalls. No packets lost.
> But now it goes through new dipsatcher prog.
> libxdp can now:
> close(dispatcher_prog_fd);
> close(link1);
> close(link2);
> Closing (and destroying two links) will remove old dispatcher prog
> from linked list in xdp_firewall1_prog->aux->linked_prog_list and from
> xdp_firewall2_prog->aux->linked_prog_list.
> Notice that there is no need to explicitly detach old dispatcher prog from eth0.
> link_update() did it while replacing it with new dispatcher prog.

Yeah, this was the flow I had in mind already. However, what I meant was
that *from the PoV of an application consuming the link fd*, this would
lead to dangling links.

I.e., an application does:

app1_link_fd = libxdp_install_prog(prog1);

and stores link_fd somewhere (just holds on to it, or pins it
somewhere).

Then later, another application does:

app2_link_fd = libxdp_install_prog(prog2);

but this has the side-effect of replacing the dispatcher, so
app1_link_fd is now no longer valid.

This can be worked around, of course (e.g., just return the prog_fd and
hide any link_fd details inside the library), but if the point of
bpf_link is that the application could hold on to it and use it for
subsequent replacements, that would be nice to have for consumers of the
library as well, no?

-Toke