netdev - Re: [PATCH bpf-next 0/3] Introduce pinnable bpf

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: <20200305163444.6e3w3u3a5ufphwhp@ast-mbp>
Date:   Thu, 5 Mar 2020 08:34:46 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next 0/3] Introduce pinnable bpf_link kernel
 abstraction

On Thu, Mar 05, 2020 at 11:37:11AM +0100, Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@...il.com> writes:
> 
> > On Wed, Mar 04, 2020 at 08:47:44AM +0100, Toke Høiland-Jørgensen wrote:
> >> >
> >> >> And what about the case where the link fd is pinned on a bpffs that is
> >> >> no longer available? I.e., if a netdevice with an XDP program moves
> >> >> namespaces and no longer has access to the original bpffs, that XDP
> >> >> program would essentially become immutable?
> >> >
> >> > 'immutable' will not be possible.
> >> > I'm not clear to me how bpffs is going to disappear. What do you mean
> >> > exactly?
> >> 
> >> # stat /sys/fs/bpf | grep Device
> >> Device: 1fh/31d	Inode: 1013963     Links: 2
> >> # mkdir /sys/fs/bpf/test; ls /sys/fs/bpf
> >> test
> >> # ip netns add test
> >> # ip netns exec test stat /sys/fs/bpf/test
> >> stat: cannot stat '/sys/fs/bpf/test': No such file or directory
> >> # ip netns exec test stat /sys/fs/bpf | grep Device
> >> Device: 3fh/63d	Inode: 12242       Links: 2
> >> 
> >> It's a different bpffs instance inside the netns, so it won't have
> >> access to anything pinned in the outer one...
> >
> > Toke, please get your facts straight.
> >
> >> # stat /sys/fs/bpf | grep Device
> >> Device: 1fh/31d	Inode: 1013963     Links: 2
> >
> > Inode != 1 means that this is not bpffs.
> > I guess this is still sysfs.
> 
> Yes, my bad; I was confused because I was misremembering when 'ip'
> mounts a new bpffs: I thought it was on every ns change, but it's only
> when loading a BPF program, and I was in a hurry so I didn't check
> properly; sorry about that.
> 
> Anyway, what I was trying to express:
> 
> > Still that doesn't mean that pinned link is 'immutable'.
> 
> I don't mean 'immutable' in the sense that it cannot be removed ever.
> Just that we may end up in a situation where an application can see a
> netdev with an XDP program attached, has the right privileges to modify
> it, but can't because it can't find the pinned bpf_link. Right? Or am I
> misunderstanding your proposal?
> 
> Amending my example from before, this could happen by:
> 
> 1. Someone attaches a program to eth0, and pins the bpf_link to
>    /sys/fs/bpf/myprog
> 
> 2. eth0 is moved to a different namespace which mounts a new sysfs at
>    /sys
> 
> 3. Inside that namespace, /sys/fs/bpf/myprog is no longer accessible, so
>    xdp-loader can't get access to the original bpf_link; but the XDP
>    program is still attached to eth0.

The key to decide is whether moving netdev across netns should be allowed
when xdp attached. I think it should be denied. Even when legacy xdp
program is attached, since it will confuse user space managing part.