[PATCH] restore behaviour of CAP_SYS_ADMIN allowing the loading of net bpf program

[PATCH] restore behaviour of CAP_SYS_ADMIN allowing the loading of net bpf program

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

This is a 5.8-rc1 regression.

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Fixes: 2c78ee898d8f ("bpf: Implement CAP_BPF")
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 kernel/bpf/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 8da159936bab..7d946435587d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2121,7 +2121,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
 	    !bpf_capable())
 		return -EPERM;
 
-	if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN))
+	if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN) && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 	if (is_perfmon_prog_type(type) && !perfmon_capable())
 		return -EPERM;
-- 
2.27.0.290.gba653c62da-goog

Re: [PATCH] restore behaviour of CAP_SYS_ADMIN allowing the loading of net bpf program

[Alexei Starovoitov]

On Thu, Jun 18, 2020 at 1:00 PM Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>
> From: Maciej Żenczykowski <maze@google.com>
>
> This is a 5.8-rc1 regression.

Please add full explanation here.

Also use [PATCH bpf] in the subject for future submission.

> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Fixes: 2c78ee898d8f ("bpf: Implement CAP_BPF")

Reported-by: John
is missing?

> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  kernel/bpf/syscall.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8da159936bab..7d946435587d 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2121,7 +2121,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
>             !bpf_capable())
>                 return -EPERM;
>
> -       if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN))
> +       if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN) && !capable(CAP_SYS_ADMIN))
>                 return -EPERM;
>         if (is_perfmon_prog_type(type) && !perfmon_capable())
>                 return -EPERM;
> --
> 2.27.0.290.gba653c62da-goog
>

[PATCH bpf v2] restore behaviour of CAP_SYS_ADMIN allowing the loading of networking bpf programs

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

This is a fix for a regression introduced in 5.8-rc1 by:
  commit 2c78ee898d8f10ae6fb2fa23a3fbaec96b1b7366
  'bpf: Implement CAP_BPF'

Before the above commit it was possible to load network bpf programs
with just the CAP_SYS_ADMIN privilege.

The Android bpfloader happens to run in such a configuration (it has
SYS_ADMIN but not NET_ADMIN) and creates maps and loads bpf programs
for later use by Android's netd (which has NET_ADMIN but not SYS_ADMIN).

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Reported-by: John Stultz <john.stultz@linaro.org>
Fixes: 2c78ee898d8f ("bpf: Implement CAP_BPF")
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 kernel/bpf/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 8da159936bab..7d946435587d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2121,7 +2121,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
 	    !bpf_capable())
 		return -EPERM;
 
-	if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN))
+	if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN) && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 	if (is_perfmon_prog_type(type) && !perfmon_capable())
 		return -EPERM;
-- 
2.27.0.111.gc72c7da667-goog

Re: [PATCH bpf v2] restore behaviour of CAP_SYS_ADMIN allowing the loading of networking bpf programs

[John Stultz]

On Sat, Jun 20, 2020 at 2:26 PM Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>
> From: Maciej Żenczykowski <maze@google.com>
>
> This is a fix for a regression introduced in 5.8-rc1 by:
>   commit 2c78ee898d8f10ae6fb2fa23a3fbaec96b1b7366
>   'bpf: Implement CAP_BPF'
>
> Before the above commit it was possible to load network bpf programs
> with just the CAP_SYS_ADMIN privilege.
>
> The Android bpfloader happens to run in such a configuration (it has
> SYS_ADMIN but not NET_ADMIN) and creates maps and loads bpf programs
> for later use by Android's netd (which has NET_ADMIN but not SYS_ADMIN).
>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Reported-by: John Stultz <john.stultz@linaro.org>
> Fixes: 2c78ee898d8f ("bpf: Implement CAP_BPF")
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Thanks so much for helping narrow this regression down and submitting this fix!
It's much appreciated!

Tested-by: John Stultz <john.stultz@linaro.org>

thanks
-john

Re: [PATCH bpf v2] restore behaviour of CAP_SYS_ADMIN allowing the loading of networking bpf programs

[Alexei Starovoitov]

On Mon, Jun 22, 2020 at 12:44 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Sat, Jun 20, 2020 at 2:26 PM Maciej Żenczykowski
> <zenczykowski@gmail.com> wrote:
> >
> > From: Maciej Żenczykowski <maze@google.com>
> >
> > This is a fix for a regression introduced in 5.8-rc1 by:
> >   commit 2c78ee898d8f10ae6fb2fa23a3fbaec96b1b7366
> >   'bpf: Implement CAP_BPF'
> >
> > Before the above commit it was possible to load network bpf programs
> > with just the CAP_SYS_ADMIN privilege.
> >
> > The Android bpfloader happens to run in such a configuration (it has
> > SYS_ADMIN but not NET_ADMIN) and creates maps and loads bpf programs
> > for later use by Android's netd (which has NET_ADMIN but not SYS_ADMIN).
> >
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Reported-by: John Stultz <john.stultz@linaro.org>
> > Fixes: 2c78ee898d8f ("bpf: Implement CAP_BPF")
> > Signed-off-by: Maciej Żenczykowski <maze@google.com>
>
> Thanks so much for helping narrow this regression down and submitting this fix!
> It's much appreciated!
>
> Tested-by: John Stultz <john.stultz@linaro.org>

Applied to bpf tree. Thanks

Re: [PATCH bpf v2] restore behaviour of CAP_SYS_ADMIN allowing the loading of networking bpf programs

[John Stultz]

On Tue, Jun 23, 2020 at 5:54 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Mon, Jun 22, 2020 at 12:44 PM John Stultz <john.stultz@linaro.org> wrote:
> > On Sat, Jun 20, 2020 at 2:26 PM Maciej Żenczykowski
> > <zenczykowski@gmail.com> wrote:
> > > From: Maciej Żenczykowski <maze@google.com>
> > >
> > > This is a fix for a regression introduced in 5.8-rc1 by:
> > >   commit 2c78ee898d8f10ae6fb2fa23a3fbaec96b1b7366
> > >   'bpf: Implement CAP_BPF'
> > >
> > > Before the above commit it was possible to load network bpf programs
> > > with just the CAP_SYS_ADMIN privilege.
> > >
> > > The Android bpfloader happens to run in such a configuration (it has
> > > SYS_ADMIN but not NET_ADMIN) and creates maps and loads bpf programs
> > > for later use by Android's netd (which has NET_ADMIN but not SYS_ADMIN).
> > >
> > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > Reported-by: John Stultz <john.stultz@linaro.org>
> > > Fixes: 2c78ee898d8f ("bpf: Implement CAP_BPF")
> > > Signed-off-by: Maciej Żenczykowski <maze@google.com>
> >
> > Thanks so much for helping narrow this regression down and submitting this fix!
> > It's much appreciated!
> >
> > Tested-by: John Stultz <john.stultz@linaro.org>
>
> Applied to bpf tree. Thanks

Hey all,
  Just wanted to follow up on this as I've not seen the regression fix
land in 5.8-rc4 yet? Is it still pending, or did it fall through a
gap?

thanks
-john

Re: [PATCH bpf v2] restore behaviour of CAP_SYS_ADMIN allowing the loading of networking bpf programs

[Daniel Borkmann]

On 7/6/20 10:11 PM, John Stultz wrote:
> On Tue, Jun 23, 2020 at 5:54 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Mon, Jun 22, 2020 at 12:44 PM John Stultz <john.stultz@linaro.org> wrote:
>>> On Sat, Jun 20, 2020 at 2:26 PM Maciej Żenczykowski
>>> <zenczykowski@gmail.com> wrote:
>>>> From: Maciej Żenczykowski <maze@google.com>
>>>>
>>>> This is a fix for a regression introduced in 5.8-rc1 by:
>>>>    commit 2c78ee898d8f10ae6fb2fa23a3fbaec96b1b7366
>>>>    'bpf: Implement CAP_BPF'
>>>>
>>>> Before the above commit it was possible to load network bpf programs
>>>> with just the CAP_SYS_ADMIN privilege.
>>>>
>>>> The Android bpfloader happens to run in such a configuration (it has
>>>> SYS_ADMIN but not NET_ADMIN) and creates maps and loads bpf programs
>>>> for later use by Android's netd (which has NET_ADMIN but not SYS_ADMIN).
>>>>
>>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>>> Reported-by: John Stultz <john.stultz@linaro.org>
>>>> Fixes: 2c78ee898d8f ("bpf: Implement CAP_BPF")
>>>> Signed-off-by: Maciej Żenczykowski <maze@google.com>
>>>
>>> Thanks so much for helping narrow this regression down and submitting this fix!
>>> It's much appreciated!
>>>
>>> Tested-by: John Stultz <john.stultz@linaro.org>
>>
>> Applied to bpf tree. Thanks
> 
> Hey all,
>    Just wanted to follow up on this as I've not seen the regression fix
> land in 5.8-rc4 yet? Is it still pending, or did it fall through a
> gap?

No, it's in DaveM's -net tree currently, will go to Linus' tree on his next pull req:

   https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=b338cb921e6739ff59ce32f43342779fe5ffa732

Re: [PATCH bpf v2] restore behaviour of CAP_SYS_ADMIN allowing the loading of networking bpf programs

[John Stultz]

On Mon, Jul 6, 2020 at 1:15 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 7/6/20 10:11 PM, John Stultz wrote:
> >    Just wanted to follow up on this as I've not seen the regression fix
> > land in 5.8-rc4 yet? Is it still pending, or did it fall through a
> > gap?
>
> No, it's in DaveM's -net tree currently, will go to Linus' tree on his next pull req:
>
>    https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=b338cb921e6739ff59ce32f43342779fe5ffa732

Great! Much appreciated! Sorry to nag!
-john