Re: [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP

["Toke Høiland-Jørgensen" <toke@redhat.com>]

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

> On Thu, Mar 26, 2020 at 01:35:13PM +0100, Toke Høiland-Jørgensen wrote:
>> 
>> Additionally, in the case where there is *not* a central management
>> daemon (i.e., what I'm implementing with libxdp), this would be the flow
>> implemented by the library without bpf_link:
>> 
>> 1. Query kernel for current BPF prog loaded on $IFACE
>> 2. Sanity-check that this program is a dispatcher program installed by
>>    libxdp
>> 3. Create a new dispatcher program with whatever changes we want to do
>>    (such as adding another component program).
>> 4. Atomically replace the old program with the new one using the netlink
>>    API in this patch series.
>
> in this model what stops another application that is not using libdispatcher to
> nuke dispatcher program ?

Nothing. But nothing is stopping it from issuing 'ip link down' either -
an application with CAP_NET_ADMIN is implicitly trusted to be
well-behaved. This patch series is just adding the kernel primitive that
enables applications to be well-behaved. I consider it an API bug-fix.

>> Whereas with bpf_link, it would be:
>> 
>> 1. Find the pinned bpf_link for $IFACE (e.g., load from
>>    /sys/fs/bpf/iface-links/$IFNAME).
>> 2. Query kernel for current BPF prog linked to $LINK
>> 3. Sanity-check that this program is a dispatcher program installed by
>>    libxdp
>> 4. Create a new dispatcher program with whatever changes we want to do
>>    (such as adding another component program).
>> 5. Atomically replace the old program with the new one using the
>>    LINK_UPDATE bpf() API.
>
> whereas here dispatcher program is only accessible to libdispatcher.
> Instance of bpffs needs to be known to libdispatcher only.
> That's the ownership I've been talking about.
>
> As discussed early we need a way for _human_ to nuke dispatcher program,
> but such api shouldn't be usable out of application/task.

As long as there is this kind of override in place, I'm not actually
fundamentally opposed to the concept of bpf_link for XDP, as an
additional mechanism. What I'm opposed to is using bpf_link as a reason
to block this series.

In fact, a way to implement the "human override" you mention, could be
to reuse the mechanism implemented in this series: If the EXPECTED_FD
passed via netlink is a bpf_link FD, that could be interpreted as an
override by the kernel.

-Toke