linux-kernel - Re: [PATCH bpf-next v6 12/23] HID: initial BPF implementation

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Jul 2022 13:43:49 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, Shuah Khan <shuah@...nel.org>,
        Dave Marchevsky <davemarchevsky@...com>,
        Joe Stringer <joe@...ium.io>, Jonathan Corbet <corbet@....net>,
        Tero Kristo <tero.kristo@...ux.intel.com>,
        lkml <linux-kernel@...r.kernel.org>,
        "open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>
Subject: Re: [PATCH bpf-next v6 12/23] HID: initial BPF implementation

On Fri, Jul 15, 2022 at 11:56:46AM +0200, Benjamin Tissoires wrote:
> On Fri, Jul 15, 2022 at 7:02 AM Greg KH <gregkh@...uxfoundation.org> wrote:
> >
> > On Tue, Jul 12, 2022 at 04:58:39PM +0200, Benjamin Tissoires wrote:
> > > --- /dev/null
> > > +++ b/drivers/hid/bpf/Kconfig
> > > @@ -0,0 +1,19 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +menu "HID-BPF support"
> > > +     #depends on x86_64
> > > +
> > > +config HID_BPF
> > > +     bool "HID-BPF support"
> > > +     default y
> >
> > Things are only default y if you can't boot your machine without it.
> > Perhaps just mirror what HID is to start with and do not select HID?
> >
> > > +     depends on BPF && BPF_SYSCALL
> > > +     select HID
> >
> > select is rough, why not depend?
> 
> Let me try to explain this mess, maybe you can give me the piece that
> I am missing:
> 
> The requirements I have (or want) are:
> - HID-BPF should be "part" of HID-core (or something similar of "part"):
>   I intend to have device fixes as part of the regular HID flow, so
> allowing distros to opt out seems a little bit dangerous
> - the HID tree is not as clean as some other trees:
>   drivers/hid/ sees both core elements and leaf drivers
>   transport layers are slightly better, they are in their own
> subdirectories, but some transport layers are everywhere in the kernel
> code or directly in drivers/hid (uhid and hid-logitech-dj for
> instance)
> - HID can be loaded as a module (only ubuntu is using that), and this
> is less and less relevant because of all of the various transport
> layers we have basically prevent a clean unloading of the module
> 
> These made me think that I should have a separate bpf subdir for
> HID-BPF, to keep things separated, which means I can not include
> HID-BPF in hid.ko directly, it goes into a separate driver. And then I
> have a chicken and egg problem:
> - HID-core needs to call functions from HID-BPF (to hook into it)
> - but HID-BPF needs to also call functions from HID-core (for
> accessing HID internals)
> 
> I have solved that situation with struct hid_bpf_ops but it is not the
> cleanest possible way.
> 
> And that's also why I did "select HID", because HID-BPF without HID is
> pointless.
> 
> One last bit I should add. hid-bpf.ko should be allowed to be compiled
> in as a module, but I had issues at boot because kfuncs were not
> getting registered properly (though it works for the net test driver).
> So I decided to make hid-bpf a boolean instead of a tristate.
> 
> As I type all of this, I am starting to wonder if I should not tackle
> the very first point and separate hid-core in its own subdir. This way
> I can have a directory with only the core part, and having hid-bpf in
> here wouldn't be too much of an issue.

We've had this problem with the USB core in the past, and yes, that was
the simplest solution (see drivers/usb/core/)

Otherwise you could do:
	default HID
as the dependancy here, but that might get messy if hid can be a module.

Try splitting the hid core out first, you want to do that anyway and
that should make this simpler as you found out :)

thanks,

greg k-h