linux-kernel - Re: [PATCH HID v2 04/13] HID: bpf: add HID-BPF hooks for hid_hw_raw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Thu, 27 Jun 2024 11:45:52 +0200
From: Benjamin Tissoires <bentiss@...nel.org>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Jiri Kosina <jikos@...nel.org>, Alexei Starovoitov <ast@...nel.org>, 
	Shuah Khan <shuah@...nel.org>, Jonathan Corbet <corbet@....net>, 
	"open list:HID CORE LAYER" <linux-input@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, bpf <bpf@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>
Subject: Re: [PATCH HID v2 04/13] HID: bpf: add HID-BPF hooks for
 hid_hw_raw_requests

On Jun 26 2024, Alexei Starovoitov wrote:
> On Wed, Jun 26, 2024 at 6:46 AM Benjamin Tissoires <bentiss@...nel.org> wrote:
> >
> > This allows to intercept and prevent or change the behavior of
> > hid_hw_raw_request() from a bpf program.
> >
> > The intent is to solve a couple of use case:
> > - firewalling a HID device: a firewall can monitor who opens the hidraw
> >   nodes and then prevent or allow access to write operations on that
> >   hidraw node.
> > - change the behavior of a device and emulate a new HID feature request
> >
> > The hook is allowed to be run as sleepable so it can itself call
> > hid_bpf_hw_request(), which allows to "convert" one feature request into
> > another or even call the feature request on a different HID device on the
> > same physical device.
> >
> > Signed-off-by: Benjamin Tissoires <bentiss@...nel.org>
> >
> > ---
> >
> > changes in v2:
> > - make use of SRCU
> > ---
> >  drivers/hid/bpf/hid_bpf_dispatch.c   | 37 ++++++++++++++++++++++++++++++++++++
> >  drivers/hid/bpf/hid_bpf_struct_ops.c |  1 +
> >  drivers/hid/hid-core.c               |  6 ++++++
> >  include/linux/hid_bpf.h              | 35 ++++++++++++++++++++++++++++++++++
> >  4 files changed, 79 insertions(+)
> >
> > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> > index c026248e3d73..ac98bab4c96d 100644
> > --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> > @@ -74,6 +74,43 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
> >  }
> >  EXPORT_SYMBOL_GPL(dispatch_hid_bpf_device_event);
> >
> > +int dispatch_hid_bpf_raw_requests(struct hid_device *hdev,
> > +                                 unsigned char reportnum, u8 *buf,
> > +                                 u32 size, enum hid_report_type rtype,
> > +                                 enum hid_class_request reqtype,
> > +                                 u64 source)
> > +{
> > +       struct hid_bpf_ctx_kern ctx_kern = {
> > +               .ctx = {
> > +                       .hid = hdev,
> > +                       .allocated_size = size,
> > +                       .size = size,
> > +               },
> > +               .data = buf,
> > +       };
> > +       struct hid_bpf_ops *e;
> > +       int ret, idx;
> > +
> > +       if (rtype >= HID_REPORT_TYPES)
> > +               return -EINVAL;
> > +
> > +       idx = srcu_read_lock(&hdev->bpf.srcu);
> > +       list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list,
> > +                                srcu_read_lock_held(&hdev->bpf.srcu)) {
> > +               if (e->hid_hw_request) {
> > +                       ret = e->hid_hw_request(&ctx_kern.ctx, reportnum, rtype, reqtype, source);
> > +                       if (ret)
> > +                               goto out;
> > +               }
> > +       }
> 
> here and in patch 7 I would reduce indent by doing:
> if (!e->hid_hw_request)
>    continue;
> ret = e->hid_hw_request(...);
> 
> otherwise lgtm

Thanks for the quick review.

I've changed the patches as you requested before applying them and also
added the Ack from Jiri he gave me over IRC.

Cheers,
Benjamin