* [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET
@ 2021-03-26 16:05 Lorenz Bauer
2021-03-26 16:05 ` [PATCH bpf v2 2/2] bpf: program: " Lorenz Bauer
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Lorenz Bauer @ 2021-03-26 16:05 UTC (permalink / raw)
To: ast, daniel, andrii; +Cc: kernel-team, netdev, bpf, Lorenz Bauer
Invoking BPF_OBJ_GET on a pinned bpf_link checks the path access
permissions based on file_flags, but the returned fd ignores flags.
This means that any user can acquire a "read-write" fd for a pinned
link with mode 0664 by invoking BPF_OBJ_GET with BPF_F_RDONLY in
file_flags. The fd can be used to invoke BPF_LINK_DETACH, etc.
Fix this by refusing non-O_RDWR flags in BPF_OBJ_GET. This works
because OBJ_GET by default returns a read write mapping and libbpf
doesn't expose a way to override this behaviour for programs
and links.
Fixes: 70ed506c3bbc ("bpf: Introduce pinnable bpf_link abstraction")
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
kernel/bpf/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 1576ff331ee4..dc56237d6960 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -547,7 +547,7 @@ int bpf_obj_get_user(const char __user *pathname, int flags)
else if (type == BPF_TYPE_MAP)
ret = bpf_map_new_fd(raw, f_flags);
else if (type == BPF_TYPE_LINK)
- ret = bpf_link_new_fd(raw);
+ ret = (f_flags != O_RDWR) ? -EINVAL : bpf_link_new_fd(raw);
else
return -ENOENT;
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf v2 2/2] bpf: program: refuse non-O_RDWR flags in BPF_OBJ_GET
2021-03-26 16:05 [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET Lorenz Bauer
@ 2021-03-26 16:05 ` Lorenz Bauer
2021-03-26 20:13 ` Song Liu
2021-03-28 4:49 ` [PATCH bpf v2 1/2] bpf: link: " Andrii Nakryiko
2021-03-31 14:04 ` Lorenz Bauer
2 siblings, 1 reply; 9+ messages in thread
From: Lorenz Bauer @ 2021-03-26 16:05 UTC (permalink / raw)
To: ast, daniel, andrii; +Cc: kernel-team, netdev, bpf, Lorenz Bauer
As for bpf_link, refuse creating a non-O_RDWR fd. Since program fds
currently don't allow modifications this is a precaution, not a
straight up bug fix.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
kernel/bpf/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index dc56237d6960..d2de2abec35b 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -543,7 +543,7 @@ int bpf_obj_get_user(const char __user *pathname, int flags)
return PTR_ERR(raw);
if (type == BPF_TYPE_PROG)
- ret = bpf_prog_new_fd(raw);
+ ret = (f_flags != O_RDWR) ? -EINVAL : bpf_prog_new_fd(raw);
else if (type == BPF_TYPE_MAP)
ret = bpf_map_new_fd(raw, f_flags);
else if (type == BPF_TYPE_LINK)
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 2/2] bpf: program: refuse non-O_RDWR flags in BPF_OBJ_GET
2021-03-26 16:05 ` [PATCH bpf v2 2/2] bpf: program: " Lorenz Bauer
@ 2021-03-26 20:13 ` Song Liu
2021-03-28 4:51 ` Andrii Nakryiko
2021-03-29 8:19 ` Lorenz Bauer
0 siblings, 2 replies; 9+ messages in thread
From: Song Liu @ 2021-03-26 20:13 UTC (permalink / raw)
To: Lorenz Bauer
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
kernel-team, Networking, bpf
On Fri, Mar 26, 2021 at 9:07 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> As for bpf_link, refuse creating a non-O_RDWR fd. Since program fds
> currently don't allow modifications this is a precaution, not a
> straight up bug fix.
>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---
> kernel/bpf/inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
> index dc56237d6960..d2de2abec35b 100644
> --- a/kernel/bpf/inode.c
> +++ b/kernel/bpf/inode.c
> @@ -543,7 +543,7 @@ int bpf_obj_get_user(const char __user *pathname, int flags)
> return PTR_ERR(raw);
For both patches, shall we do the check before bpf_obj_do_get(), which is a few
lines above?
Thanks,
Song
>
> if (type == BPF_TYPE_PROG)
> - ret = bpf_prog_new_fd(raw);
> + ret = (f_flags != O_RDWR) ? -EINVAL : bpf_prog_new_fd(raw);
> else if (type == BPF_TYPE_MAP)
> ret = bpf_map_new_fd(raw, f_flags);
> else if (type == BPF_TYPE_LINK)
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET
2021-03-26 16:05 [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET Lorenz Bauer
2021-03-26 16:05 ` [PATCH bpf v2 2/2] bpf: program: " Lorenz Bauer
@ 2021-03-28 4:49 ` Andrii Nakryiko
2021-03-31 14:04 ` Lorenz Bauer
2 siblings, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2021-03-28 4:49 UTC (permalink / raw)
To: Lorenz Bauer
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
kernel-team, Networking, bpf
On Fri, Mar 26, 2021 at 9:05 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> Invoking BPF_OBJ_GET on a pinned bpf_link checks the path access
> permissions based on file_flags, but the returned fd ignores flags.
> This means that any user can acquire a "read-write" fd for a pinned
> link with mode 0664 by invoking BPF_OBJ_GET with BPF_F_RDONLY in
> file_flags. The fd can be used to invoke BPF_LINK_DETACH, etc.
>
> Fix this by refusing non-O_RDWR flags in BPF_OBJ_GET. This works
> because OBJ_GET by default returns a read write mapping and libbpf
> doesn't expose a way to override this behaviour for programs
> and links.
>
> Fixes: 70ed506c3bbc ("bpf: Introduce pinnable bpf_link abstraction")
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---
LGTM.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> kernel/bpf/inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
> index 1576ff331ee4..dc56237d6960 100644
> --- a/kernel/bpf/inode.c
> +++ b/kernel/bpf/inode.c
> @@ -547,7 +547,7 @@ int bpf_obj_get_user(const char __user *pathname, int flags)
> else if (type == BPF_TYPE_MAP)
> ret = bpf_map_new_fd(raw, f_flags);
> else if (type == BPF_TYPE_LINK)
> - ret = bpf_link_new_fd(raw);
> + ret = (f_flags != O_RDWR) ? -EINVAL : bpf_link_new_fd(raw);
> else
> return -ENOENT;
>
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 2/2] bpf: program: refuse non-O_RDWR flags in BPF_OBJ_GET
2021-03-26 20:13 ` Song Liu
@ 2021-03-28 4:51 ` Andrii Nakryiko
2021-03-29 8:19 ` Lorenz Bauer
1 sibling, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2021-03-28 4:51 UTC (permalink / raw)
To: Song Liu
Cc: Lorenz Bauer, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, kernel-team, Networking, bpf
On Fri, Mar 26, 2021 at 1:14 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Mar 26, 2021 at 9:07 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
> >
> > As for bpf_link, refuse creating a non-O_RDWR fd. Since program fds
> > currently don't allow modifications this is a precaution, not a
> > straight up bug fix.
> >
> > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> > ---
> > kernel/bpf/inode.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
> > index dc56237d6960..d2de2abec35b 100644
> > --- a/kernel/bpf/inode.c
> > +++ b/kernel/bpf/inode.c
> > @@ -543,7 +543,7 @@ int bpf_obj_get_user(const char __user *pathname, int flags)
> > return PTR_ERR(raw);
>
> For both patches, shall we do the check before bpf_obj_do_get(), which is a few
> lines above?
Map does use f_flags, so we need to let them through. Or did you mean
to do a (type != BPF_TYPE_MAP && f_flags != O_RDWR) check?
Either way is fine with me, so:
Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> Thanks,
> Song
>
> >
> > if (type == BPF_TYPE_PROG)
> > - ret = bpf_prog_new_fd(raw);
> > + ret = (f_flags != O_RDWR) ? -EINVAL : bpf_prog_new_fd(raw);
> > else if (type == BPF_TYPE_MAP)
> > ret = bpf_map_new_fd(raw, f_flags);
> > else if (type == BPF_TYPE_LINK)
> > --
> > 2.27.0
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 2/2] bpf: program: refuse non-O_RDWR flags in BPF_OBJ_GET
2021-03-26 20:13 ` Song Liu
2021-03-28 4:51 ` Andrii Nakryiko
@ 2021-03-29 8:19 ` Lorenz Bauer
1 sibling, 0 replies; 9+ messages in thread
From: Lorenz Bauer @ 2021-03-29 8:19 UTC (permalink / raw)
To: Song Liu
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
kernel-team, Networking, bpf
On Fri, 26 Mar 2021 at 20:14, Song Liu <song@kernel.org> wrote:
>
> On Fri, Mar 26, 2021 at 9:07 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
> >
> > As for bpf_link, refuse creating a non-O_RDWR fd. Since program fds
> > currently don't allow modifications this is a precaution, not a
> > straight up bug fix.
> >
> > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> > ---
> > kernel/bpf/inode.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
> > index dc56237d6960..d2de2abec35b 100644
> > --- a/kernel/bpf/inode.c
> > +++ b/kernel/bpf/inode.c
> > @@ -543,7 +543,7 @@ int bpf_obj_get_user(const char __user *pathname, int flags)
> > return PTR_ERR(raw);
>
> For both patches, shall we do the check before bpf_obj_do_get(), which is a few
> lines above?
type is filled in by bpf_obj_do_get, so we can't avoid calling it. As
Andrii mentions we need to allow flags for map.
>
> Thanks,
> Song
>
> >
> > if (type == BPF_TYPE_PROG)
> > - ret = bpf_prog_new_fd(raw);
> > + ret = (f_flags != O_RDWR) ? -EINVAL : bpf_prog_new_fd(raw);
> > else if (type == BPF_TYPE_MAP)
> > ret = bpf_map_new_fd(raw, f_flags);
> > else if (type == BPF_TYPE_LINK)
> > --
> > 2.27.0
> >
--
Lorenz Bauer | Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
www.cloudflare.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET
2021-03-26 16:05 [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET Lorenz Bauer
2021-03-26 16:05 ` [PATCH bpf v2 2/2] bpf: program: " Lorenz Bauer
2021-03-28 4:49 ` [PATCH bpf v2 1/2] bpf: link: " Andrii Nakryiko
@ 2021-03-31 14:04 ` Lorenz Bauer
2021-04-01 18:04 ` Alexei Starovoitov
2021-04-01 21:44 ` Alexei Starovoitov
2 siblings, 2 replies; 9+ messages in thread
From: Lorenz Bauer @ 2021-03-31 14:04 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: kernel-team, Networking, bpf
On Fri, 26 Mar 2021 at 16:05, Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> Invoking BPF_OBJ_GET on a pinned bpf_link checks the path access
> permissions based on file_flags, but the returned fd ignores flags.
> This means that any user can acquire a "read-write" fd for a pinned
> link with mode 0664 by invoking BPF_OBJ_GET with BPF_F_RDONLY in
> file_flags. The fd can be used to invoke BPF_LINK_DETACH, etc.
>
> Fix this by refusing non-O_RDWR flags in BPF_OBJ_GET. This works
> because OBJ_GET by default returns a read write mapping and libbpf
> doesn't expose a way to override this behaviour for programs
> and links.
Hi Alexei and Daniel,
I think these two patches might have fallen through the cracks, could
you take a look? I'm not sure what the etiquette is around bumping a
set, so please let me know if you'd prefer me to send the patches with
acks included or something like that.
Best
--
Lorenz Bauer | Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
www.cloudflare.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET
2021-03-31 14:04 ` Lorenz Bauer
@ 2021-04-01 18:04 ` Alexei Starovoitov
2021-04-01 21:44 ` Alexei Starovoitov
1 sibling, 0 replies; 9+ messages in thread
From: Alexei Starovoitov @ 2021-04-01 18:04 UTC (permalink / raw)
To: Lorenz Bauer
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team, Networking, bpf
On Wed, Mar 31, 2021 at 7:04 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> On Fri, 26 Mar 2021 at 16:05, Lorenz Bauer <lmb@cloudflare.com> wrote:
> >
> > Invoking BPF_OBJ_GET on a pinned bpf_link checks the path access
> > permissions based on file_flags, but the returned fd ignores flags.
> > This means that any user can acquire a "read-write" fd for a pinned
> > link with mode 0664 by invoking BPF_OBJ_GET with BPF_F_RDONLY in
> > file_flags. The fd can be used to invoke BPF_LINK_DETACH, etc.
> >
> > Fix this by refusing non-O_RDWR flags in BPF_OBJ_GET. This works
> > because OBJ_GET by default returns a read write mapping and libbpf
> > doesn't expose a way to override this behaviour for programs
> > and links.
>
> Hi Alexei and Daniel,
>
> I think these two patches might have fallen through the cracks, could
> you take a look? I'm not sure what the etiquette is around bumping a
> set, so please let me know if you'd prefer me to send the patches with
> acks included or something like that.
It is still in patchworks. I didn't have time to think it through.
Sorry for delay.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET
2021-03-31 14:04 ` Lorenz Bauer
2021-04-01 18:04 ` Alexei Starovoitov
@ 2021-04-01 21:44 ` Alexei Starovoitov
1 sibling, 0 replies; 9+ messages in thread
From: Alexei Starovoitov @ 2021-04-01 21:44 UTC (permalink / raw)
To: Lorenz Bauer
Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team, Networking, bpf
On Wed, Mar 31, 2021 at 7:04 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> On Fri, 26 Mar 2021 at 16:05, Lorenz Bauer <lmb@cloudflare.com> wrote:
> >
> > Invoking BPF_OBJ_GET on a pinned bpf_link checks the path access
> > permissions based on file_flags, but the returned fd ignores flags.
> > This means that any user can acquire a "read-write" fd for a pinned
> > link with mode 0664 by invoking BPF_OBJ_GET with BPF_F_RDONLY in
> > file_flags. The fd can be used to invoke BPF_LINK_DETACH, etc.
> >
> > Fix this by refusing non-O_RDWR flags in BPF_OBJ_GET. This works
> > because OBJ_GET by default returns a read write mapping and libbpf
> > doesn't expose a way to override this behaviour for programs
> > and links.
>
> Hi Alexei and Daniel,
>
> I think these two patches might have fallen through the cracks, could
> you take a look? I'm not sure what the etiquette is around bumping a
> set, so please let me know if you'd prefer me to send the patches with
> acks included or something like that.
Applied both. Thanks
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-04-01 21:44 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26 16:05 [PATCH bpf v2 1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET Lorenz Bauer
2021-03-26 16:05 ` [PATCH bpf v2 2/2] bpf: program: " Lorenz Bauer
2021-03-26 20:13 ` Song Liu
2021-03-28 4:51 ` Andrii Nakryiko
2021-03-29 8:19 ` Lorenz Bauer
2021-03-28 4:49 ` [PATCH bpf v2 1/2] bpf: link: " Andrii Nakryiko
2021-03-31 14:04 ` Lorenz Bauer
2021-04-01 18:04 ` Alexei Starovoitov
2021-04-01 21:44 ` Alexei Starovoitov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.