Re: Advisory file locking behaviour of bpf_link (and others?)

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

Lorenz Bauer <lmb@cloudflare.com> writes:

> On Tue, 25 Aug 2020 at 19:06, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Tue, Aug 25, 2020 at 6:39 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>> >
>> > Hi,
>> >
>> > I was playing around a bit, and noticed that trying to acquire an
>> > exclusive POSIX record lock on a bpf_link fd fails. I've traced this
>> > to the call to anon_inode_getfile from bpf_link_prime which
>> > effectively specifies O_RDONLY on the bpf_link struct file. This makes
>> > check_fmode_for_setlk return EBADF.
>> >
>> > This means the following:
>> > * flock(link, LOCK_EX): works
>> > * fcntl(link, SETLK, F_RDLCK): works
>> > * fcntl(link, SETLK, F_WRLCK): doesn't work
>> >
>> > Especially the discrepancy between flock(EX) and fcntl(WRLCK) has me
>> > puzzled. Should fcntl(WRLCK) work on a link?
>> >
>> > program fds are always O_RDWR as far as I can tell (so all locks
>> > work), while maps depend on map_flags.
>>
>> Because for links fd/file flags are reserved for the future use.
>> progs are rdwr for historical reasons while maps can have three combinations:
>> /* Flags for accessing BPF object from syscall side. */
>>         BPF_F_RDONLY            = (1U << 3),
>>         BPF_F_WRONLY            = (1U << 4),
>> by default they are rdwr.
>> What is your use case to use flock on bpf_link fd?
>
> The idea is to prevent concurrent access / modification of pinned maps
> + pinned link from a command line tool. I could just as well lock one
> of the maps for this, but conceptually the link is the thing that
> actually controls what maps are used via the attached BPF program.
> FWIW I'm using flock(EX) on the link for now, which is fine for my use
> case. I just thought I'd raise this in case it was an oversight :)

FWIW I'm doing something similar in libxdp, except I'm using flock(EX)
on the parent directory (i.e., /sys/fs/bpf/xdp) since I need to protect
multiple modifications inside it:

https://github.com/xdp-project/xdp-tools/blob/master/lib/libxdp/libxdp.c#L245

-Toke