netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf: skip unnecessary capability check
@ 2018-03-20  0:57 Chenbo Feng
  2018-03-20 11:37 ` Lorenzo Colitti
  0 siblings, 1 reply; 3+ messages in thread
From: Chenbo Feng @ 2018-03-20  0:57 UTC (permalink / raw)
  To: netdev, ast; +Cc: Jeffrey Vander Stoep, lorenzo, Daniel Borkmann, Chenbo Feng

From: Chenbo Feng <fengc@google.com>

The current check statement in BPF syscall will do a capability check
for CAP_SYS_ADMIN before checking sysctl_unprivileged_bpf_disabled. This
code path will trigger unnecessary security hooks on capability checking
and cause false alarms on unprivileged process trying to get CAP_SYS_ADMIN
access. This can be resolved by simply switch the order of the statement
and CAP_SYS_ADMIN is not required anyway if unprivileged bpf syscall is
allowed.

Signed-off-by: Chenbo Feng <fengc@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 e24aa3241387..43f95d190eea 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1845,7 +1845,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 	union bpf_attr attr = {};
 	int err;
 
-	if (!capable(CAP_SYS_ADMIN) && sysctl_unprivileged_bpf_disabled)
+	if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	err = check_uarg_tail_zero(uattr, sizeof(attr), size);
-- 
2.16.2.804.g6dcf76e118-goog

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] bpf: skip unnecessary capability check
  2018-03-20  0:57 [PATCH bpf-next] bpf: skip unnecessary capability check Chenbo Feng
@ 2018-03-20 11:37 ` Lorenzo Colitti
  2018-03-20 22:55   ` Daniel Borkmann
  0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Colitti @ 2018-03-20 11:37 UTC (permalink / raw)
  To: Chenbo Feng
  Cc: netdev, ast, Jeffrey Vander Stoep, Daniel Borkmann, Chenbo Feng

On Tue, Mar 20, 2018 at 12:57 AM, Chenbo Feng
<chenbofeng.kernel@gmail.com> wrote:
> -       if (!capable(CAP_SYS_ADMIN) && sysctl_unprivileged_bpf_disabled)
> +       if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
>                 return -EPERM;
>

Acked-by: Lorenzo Colitti <lorenzo@google.com>

Should this be targeted to bpf (or even -stable) instead of bpf-next?

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] bpf: skip unnecessary capability check
  2018-03-20 11:37 ` Lorenzo Colitti
@ 2018-03-20 22:55   ` Daniel Borkmann
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2018-03-20 22:55 UTC (permalink / raw)
  To: Lorenzo Colitti, Chenbo Feng
  Cc: netdev, ast, Jeffrey Vander Stoep, Chenbo Feng

On 03/20/2018 12:37 PM, Lorenzo Colitti wrote:
> On Tue, Mar 20, 2018 at 12:57 AM, Chenbo Feng
> <chenbofeng.kernel@gmail.com> wrote:
>> -       if (!capable(CAP_SYS_ADMIN) && sysctl_unprivileged_bpf_disabled)
>> +       if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
>>                 return -EPERM;
>>
> 
> Acked-by: Lorenzo Colitti <lorenzo@google.com>
> 
> Should this be targeted to bpf (or even -stable) instead of bpf-next?

Ok, I've applied to bpf tree, thanks guys!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-03-20 22:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-20  0:57 [PATCH bpf-next] bpf: skip unnecessary capability check Chenbo Feng
2018-03-20 11:37 ` Lorenzo Colitti
2018-03-20 22:55   ` Daniel Borkmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).