| Message ID | ZDWAcN6wfeXzipHz@gofer.mess.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bpf: lirc program type should not require SYS_CAP_ADMIN | expand |
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 >
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 > >
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?
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
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?
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
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:
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(-)