diff mbox series

[bpf,v2,2/2] bpf: program: refuse non-O_RDWR flags in BPF_OBJ_GET

Message ID 20210326160501.46234-2-lmb@cloudflare.com
State New
Headers show
Series [bpf,v2,1/2] bpf: link: refuse non-O_RDWR flags in BPF_OBJ_GET | expand

Commit Message

Lorenz Bauer March 26, 2021, 4:05 p.m. UTC
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(-)

Comments

Andrii Nakryiko March 28, 2021, 4:51 a.m. UTC | #1
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

> >
Lorenz Bauer March 29, 2021, 8:19 a.m. UTC | #2
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
diff mbox series

Patch

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)