[PATCH] bpf: lirc program type should not require SYS_CAP_ADMIN

[PATCH] bpf: lirc program type should not require SYS_CAP_ADMIN

[Sean Young]

Make it possible to load lirc program type with just CAP_BPF.

Signed-off-by: Sean Young <sean@mess.org>
---
 kernel/bpf/syscall.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index adc83cb82f37..19d9265270b3 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2439,7 +2439,6 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type)
 	case BPF_PROG_TYPE_LWT_SEG6LOCAL:
 	case BPF_PROG_TYPE_SK_SKB:
 	case BPF_PROG_TYPE_SK_MSG:
-	case BPF_PROG_TYPE_LIRC_MODE2:
 	case BPF_PROG_TYPE_FLOW_DISSECTOR:
 	case BPF_PROG_TYPE_CGROUP_DEVICE:
 	case BPF_PROG_TYPE_CGROUP_SOCK:
-- 
2.39.2

Re: [PATCH] bpf: lirc program type should not require SYS_CAP_ADMIN

[Alexei Starovoitov]

On Tue, Apr 11, 2023 at 8:45 AM Sean Young <sean@mess.org> wrote:
>
> Make it possible to load lirc program type with just CAP_BPF.

Is it safe?
If the user can load with just CAP_BPF the FD to the prog and target_fd
will allow attach as well.


> Signed-off-by: Sean Young <sean@mess.org>
> ---
>  kernel/bpf/syscall.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index adc83cb82f37..19d9265270b3 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2439,7 +2439,6 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type)
>         case BPF_PROG_TYPE_LWT_SEG6LOCAL:
>         case BPF_PROG_TYPE_SK_SKB:
>         case BPF_PROG_TYPE_SK_MSG:
> -       case BPF_PROG_TYPE_LIRC_MODE2:
>         case BPF_PROG_TYPE_FLOW_DISSECTOR:
>         case BPF_PROG_TYPE_CGROUP_DEVICE:
>         case BPF_PROG_TYPE_CGROUP_SOCK:
> --
> 2.39.2
>

Re: [PATCH] bpf: lirc program type should not require SYS_CAP_ADMIN

[Sean Young]

On Wed, Apr 12, 2023 at 04:14:05PM -0700, Alexei Starovoitov wrote:
> On Tue, Apr 11, 2023 at 8:45 AM Sean Young <sean@mess.org> wrote:
> >
> > Make it possible to load lirc program type with just CAP_BPF.
> 
> Is it safe?
> If the user can load with just CAP_BPF the FD to the prog and target_fd
> will allow attach as well.

Exactly, that's the $1m question of course. 

I think it's safe from a lirc perspective because you need to be able to
open the /dev/lirc0 device in the first place; if you can open it, you
alter all sorts of lirc receiving options already. Changing the IR protocol
decoder is no different in that perspective.

The other side of course, is it save to load a bpf lirc program as a normal
user. I don't see any issue with this; I guess this depends on whether the
subset of functions in lirc_mode2_func_proto() is safe. I am hoping that
the expert opinion everyone here can help answer that question.

Thanks,

Sean

> 
> 
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> >  kernel/bpf/syscall.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index adc83cb82f37..19d9265270b3 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -2439,7 +2439,6 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type)
> >         case BPF_PROG_TYPE_LWT_SEG6LOCAL:
> >         case BPF_PROG_TYPE_SK_SKB:
> >         case BPF_PROG_TYPE_SK_MSG:
> > -       case BPF_PROG_TYPE_LIRC_MODE2:
> >         case BPF_PROG_TYPE_FLOW_DISSECTOR:
> >         case BPF_PROG_TYPE_CGROUP_DEVICE:
> >         case BPF_PROG_TYPE_CGROUP_SOCK:
> > --
> > 2.39.2
> >

Re: [PATCH] bpf: lirc program type should not require SYS_CAP_ADMIN

[Alexei Starovoitov]

On Thu, Apr 13, 2023 at 1:28 AM Sean Young <sean@mess.org> wrote:
>
> On Wed, Apr 12, 2023 at 04:14:05PM -0700, Alexei Starovoitov wrote:
> > On Tue, Apr 11, 2023 at 8:45 AM Sean Young <sean@mess.org> wrote:
> > >
> > > Make it possible to load lirc program type with just CAP_BPF.
> >
> > Is it safe?
> > If the user can load with just CAP_BPF the FD to the prog and target_fd
> > will allow attach as well.
>
> Exactly, that's the $1m question of course.
>
> I think it's safe from a lirc perspective because you need to be able to
> open the /dev/lirc0 device in the first place; if you can open it, you
> alter all sorts of lirc receiving options already. Changing the IR protocol
> decoder is no different in that perspective.
>
> The other side of course, is it save to load a bpf lirc program as a normal
> user. I don't see any issue with this; I guess this depends on whether the
> subset of functions in lirc_mode2_func_proto() is safe. I am hoping that
> the expert opinion everyone here can help answer that question.

That part is fine under CAP_BPF.
I don't know how lirc devices are typically setup.
If they need root to open them
then why bother relaxing bpf loading part?

Re: [PATCH] bpf: lirc program type should not require SYS_CAP_ADMIN

[Sean Young]

On Thu, Apr 13, 2023 at 04:54:21PM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 13, 2023 at 1:28 AM Sean Young <sean@mess.org> wrote:
> >
> > On Wed, Apr 12, 2023 at 04:14:05PM -0700, Alexei Starovoitov wrote:
> > > On Tue, Apr 11, 2023 at 8:45 AM Sean Young <sean@mess.org> wrote:
> > > >
> > > > Make it possible to load lirc program type with just CAP_BPF.
> > >
> > > Is it safe?
> > > If the user can load with just CAP_BPF the FD to the prog and target_fd
> > > will allow attach as well.
> >
> > Exactly, that's the $1m question of course.
> >
> > I think it's safe from a lirc perspective because you need to be able to
> > open the /dev/lirc0 device in the first place; if you can open it, you
> > alter all sorts of lirc receiving options already. Changing the IR protocol
> > decoder is no different in that perspective.
> >
> > The other side of course, is it save to load a bpf lirc program as a normal
> > user. I don't see any issue with this; I guess this depends on whether the
> > subset of functions in lirc_mode2_func_proto() is safe. I am hoping that
> > the expert opinion everyone here can help answer that question.
> 
> That part is fine under CAP_BPF.
> I don't know how lirc devices are typically setup.
> If they need root to open them
> then why bother relaxing bpf loading part?

I'd like to get a point where /dev/lircN can have the same permissions as
for example /dev/videoN devices: group read/write, so that local users
don't have to become root to use them.

Without relaxing the bpf side, this seems like a chicken and egg problem
(tiktaalik and egg?).

Also - the CAP_NET_ADMIN requirement seems completely arbitrary compared
to other program types.


Sean

Re: [PATCH] bpf: lirc program type should not require SYS_CAP_ADMIN

[Alexei Starovoitov]

On Fri, Apr 14, 2023 at 3:58 AM Sean Young <sean@mess.org> wrote:
>
> On Thu, Apr 13, 2023 at 04:54:21PM -0700, Alexei Starovoitov wrote:
> > On Thu, Apr 13, 2023 at 1:28 AM Sean Young <sean@mess.org> wrote:
> > >
> > > On Wed, Apr 12, 2023 at 04:14:05PM -0700, Alexei Starovoitov wrote:
> > > > On Tue, Apr 11, 2023 at 8:45 AM Sean Young <sean@mess.org> wrote:
> > > > >
> > > > > Make it possible to load lirc program type with just CAP_BPF.
> > > >
> > > > Is it safe?
> > > > If the user can load with just CAP_BPF the FD to the prog and target_fd
> > > > will allow attach as well.
> > >
> > > Exactly, that's the $1m question of course.
> > >
> > > I think it's safe from a lirc perspective because you need to be able to
> > > open the /dev/lirc0 device in the first place; if you can open it, you
> > > alter all sorts of lirc receiving options already. Changing the IR protocol
> > > decoder is no different in that perspective.
> > >
> > > The other side of course, is it save to load a bpf lirc program as a normal
> > > user. I don't see any issue with this; I guess this depends on whether the
> > > subset of functions in lirc_mode2_func_proto() is safe. I am hoping that
> > > the expert opinion everyone here can help answer that question.
> >
> > That part is fine under CAP_BPF.
> > I don't know how lirc devices are typically setup.
> > If they need root to open them
> > then why bother relaxing bpf loading part?
>
> I'd like to get a point where /dev/lircN can have the same permissions as
> for example /dev/videoN devices: group read/write, so that local users
> don't have to become root to use them.
>
> Without relaxing the bpf side, this seems like a chicken and egg problem
> (tiktaalik and egg?).
>
> Also - the CAP_NET_ADMIN requirement seems completely arbitrary compared
> to other program types.

Yeah. Agree. Could you respin with all these additional details
explaining the motivation and driver permission model?

Re: [PATCH] bpf: lirc program type should not require SYS_CAP_ADMIN

[Sean Young]

On Fri, Apr 14, 2023 at 08:39:13AM -0700, Alexei Starovoitov wrote:
> Yeah. Agree. Could you respin with all these additional details
> explaining the motivation and driver permission model?

Thanks, will do. The commit message was way too short.


Sean