Advisory file locking behaviour of bpf_link (and others?)

Advisory file locking behaviour of bpf_link (and others?)

[Lorenz Bauer]

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.

Best
Lorenz

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

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

[Alexei Starovoitov]

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?

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

[Lorenz Bauer]

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 :)

Best
Lorenz

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

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

[Toke Høiland-Jørgensen]

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

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

[Lorenz Bauer]

On Wed, 26 Aug 2020 at 10:22, Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> 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:

Thank you for the suggestion, that is indeed much nicer! Now why did I
bother with fcntl in the first place? :)

Best

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com