| Message ID | 20220318161528.1531164-1-benjamin.tissoires@redhat.com |
|---|---|
| Headers | show |
| Series | Introduce eBPF support for HID devices | expand |
On Fri, Mar 18, 2022 at 9:22 AM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > Gives a primer on HID-BPF. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > new in v3 > --- > Documentation/hid/hid-bpf.rst | 444 ++++++++++++++++++++++++++++++++++ > Documentation/hid/index.rst | 1 + > include/uapi/linux/bpf_hid.h | 54 ++++- > 3 files changed, 492 insertions(+), 7 deletions(-) > create mode 100644 Documentation/hid/hid-bpf.rst > > diff --git a/Documentation/hid/hid-bpf.rst b/Documentation/hid/hid-bpf.rst > new file mode 100644 > index 000000000000..0bf0d937b0e1 > --- /dev/null > +++ b/Documentation/hid/hid-bpf.rst > @@ -0,0 +1,444 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +======= > +HID-BPF > +======= > + > +HID is a standard protocol for input devices and it can greatly make use > +of the eBPF capabilities to speed up development and add new capabilities > +to the existing HID interfaces. > + > +.. contents:: > + :local: > + :depth: 2 > + > + > +When (and why) using HID-BPF > +============================ > + > +We can enumerate several use cases for when using HID-BPF is better than > +using a standard kernel driver fix. > + > +dead zone of a joystick > +----------------------- > + > +Assuming you have a joystick that is getting older, it is common to see it > +wobbling around its neutral point. This is usually filtered at the application > +level by adding a *dead zone* for this specific axis. > + > +With HID-BPF, we can put the filtering of this dead zone in the kernel directly > +so we don't wake up userspace when nothing else is happening on the input > +controller. > + > +Of course, given that this dead zone is device specific, we can not create a nit: s/can not/cannot There are a few more "can not" below. [...] > +firewall > +-------- > + > +What if we want to prevent other users to access a specific feature of a > +device? (think a possibly bonker firmware update entry popint) nit: point > + > +With eBPF, we can intercept any HID command emitted to the device and > +validate it or not. > + > +This also allows to sync the state between the userspace and the > +kernel/bpf program because we can intercept any incoming command. > + [...] > + > +The main idea behind HID-BPF is that it works at an array of bytes level. > +Thus, all of the parsing of the HID report and the HID report descriptor > +must be implemented in the userspace component that loads the eBPF > +program. > + > +For example, in the dead zone joystick from above, knowing which fields > +in the data stream needs to be set to ``0`` needs to be computed by userspace. > + > +A corrolar of this is that HID-BPF doesn't know about the other subsystems nit: corollary? > +available in the kernel. *You can not directly emit input event through the > +input API from eBPF*. > + > +When a BPF program need to emit input events, it needs to talk HID, and rely > +on the HID kernel processing to translate the HID data into input events. > + > +Available types of programs > +=========================== [...] > + > +``BPF_HID_RDESC_FIXUP`` > +~~~~~~~~~~~~~~~~~~~~~~~ > + > +Last, the ``BPF_HID_RDESC_FIXUP`` program works in the similar maneer than nit: manner. > +``.report_fixup`` of ``struct hid_driver``. > + [...]
On Fri, Mar 18, 2022 at 9:18 AM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > HID-bpf program type are needing new SECs. > To bind a hid-bpf program, we can rely on bpf_program__attach_fd() > so export a new function to the API. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Acked-by: Song Liu <songliubraving@fb.com> > > --- > > changes in v3: > - squashed the libbpf changes into 1 > - moved bpf_program__attach_hid to 0.8.0 > - added SEC_DEF("hid/driver_event") > > changes in v2: > - split the series by bpf/libbpf/hid/selftests and samples > --- > tools/include/uapi/linux/bpf.h | 31 +++++++++++++++++++++++++++++++ > tools/lib/bpf/libbpf.c | 23 +++++++++++++++++------ > tools/lib/bpf/libbpf.h | 2 ++ > tools/lib/bpf/libbpf.map | 1 + > 4 files changed, 51 insertions(+), 6 deletions(-) > > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index 99fab54ae9c0..0e8438e93768 100644 > --- a/tools/include/uapi/linux/bpf.h > +++ b/tools/include/uapi/linux/bpf.h > @@ -952,6 +952,7 @@ enum bpf_prog_type { > BPF_PROG_TYPE_LSM, > BPF_PROG_TYPE_SK_LOOKUP, > BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */ > + BPF_PROG_TYPE_HID, > }; > > enum bpf_attach_type { > @@ -997,6 +998,10 @@ enum bpf_attach_type { > BPF_SK_REUSEPORT_SELECT, > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, > BPF_PERF_EVENT, > + BPF_HID_DEVICE_EVENT, > + BPF_HID_RDESC_FIXUP, > + BPF_HID_USER_EVENT, > + BPF_HID_DRIVER_EVENT, > __MAX_BPF_ATTACH_TYPE > }; > > @@ -1011,6 +1016,7 @@ enum bpf_link_type { > BPF_LINK_TYPE_NETNS = 5, > BPF_LINK_TYPE_XDP = 6, > BPF_LINK_TYPE_PERF_EVENT = 7, > + BPF_LINK_TYPE_HID = 8, > > MAX_BPF_LINK_TYPE, > }; > @@ -1118,6 +1124,16 @@ enum bpf_link_type { > */ > #define BPF_F_XDP_HAS_FRAGS (1U << 5) > > +/* HID flag used in BPF_LINK_CREATE command > + * > + * NONE(default): The bpf program will be added at the tail of the list > + * of existing bpf program for this type. > + * > + * BPF_F_INSERT_HEAD: The bpf program will be added at the beginning > + * of the list of existing bpf program for this type.. > + */ > +#define BPF_F_INSERT_HEAD (1U << 0) > + > /* When BPF ldimm64's insn[0].src_reg != 0 then this can have > * the following extensions: > * > @@ -5129,6 +5145,16 @@ union bpf_attr { > * The **hash_algo** is returned on success, > * **-EOPNOTSUP** if the hash calculation failed or **-EINVAL** if > * invalid arguments are passed. > + * > + * void *bpf_hid_get_data(void *ctx, u64 offset, u64 size) > + * Description > + * Returns a pointer to the data associated with context at the given > + * offset and size (in bytes). > + * > + * Note: the returned pointer is refcounted and must be dereferenced > + * by a call to bpf_hid_discard; > + * Return > + * The pointer to the data. On error, a null value is returned. > */ > #define __BPF_FUNC_MAPPER(FN) \ > FN(unspec), \ > @@ -5325,6 +5351,7 @@ union bpf_attr { > FN(copy_from_user_task), \ > FN(skb_set_tstamp), \ > FN(ima_file_hash), \ > + FN(hid_get_data), \ > /* */ > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > @@ -5925,6 +5952,10 @@ struct bpf_link_info { > struct { > __u32 ifindex; > } xdp; > + struct { > + __s32 hidraw_number; > + __u32 attach_type; > + } hid; > }; > } __attribute__((aligned(8))); > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 43161fdd44bb..6b9ba313eb5b 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -8675,6 +8675,10 @@ static const struct bpf_sec_def section_defs[] = { > SEC_DEF("cgroup/setsockopt", CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > SEC_DEF("struct_ops+", STRUCT_OPS, 0, SEC_NONE), > SEC_DEF("sk_lookup", SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE | SEC_SLOPPY_PFX), > + SEC_DEF("hid/device_event", HID, BPF_HID_DEVICE_EVENT, SEC_ATTACHABLE_OPT), > + SEC_DEF("hid/rdesc_fixup", HID, BPF_HID_RDESC_FIXUP, SEC_ATTACHABLE_OPT), > + SEC_DEF("hid/user_event", HID, BPF_HID_USER_EVENT, SEC_ATTACHABLE_OPT), > + SEC_DEF("hid/driver_event", HID, BPF_HID_DRIVER_EVENT, SEC_ATTACHABLE_OPT), > }; > > static size_t custom_sec_def_cnt; > @@ -10630,10 +10634,11 @@ static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_li > > static struct bpf_link * > bpf_program__attach_fd(const struct bpf_program *prog, int target_fd, int btf_id, > - const char *target_name) > + const char *target_name, __u32 flags) > { > DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts, > - .target_btf_id = btf_id); > + .target_btf_id = btf_id, > + .flags = flags); > enum bpf_attach_type attach_type; > char errmsg[STRERR_BUFSIZE]; > struct bpf_link *link; > @@ -10667,19 +10672,19 @@ bpf_program__attach_fd(const struct bpf_program *prog, int target_fd, int btf_id > struct bpf_link * > bpf_program__attach_cgroup(const struct bpf_program *prog, int cgroup_fd) > { > - return bpf_program__attach_fd(prog, cgroup_fd, 0, "cgroup"); > + return bpf_program__attach_fd(prog, cgroup_fd, 0, "cgroup", 0); > } > > struct bpf_link * > bpf_program__attach_netns(const struct bpf_program *prog, int netns_fd) > { > - return bpf_program__attach_fd(prog, netns_fd, 0, "netns"); > + return bpf_program__attach_fd(prog, netns_fd, 0, "netns", 0); > } > > struct bpf_link *bpf_program__attach_xdp(const struct bpf_program *prog, int ifindex) > { > /* target_fd/target_ifindex use the same field in LINK_CREATE */ > - return bpf_program__attach_fd(prog, ifindex, 0, "xdp"); > + return bpf_program__attach_fd(prog, ifindex, 0, "xdp", 0); > } > > struct bpf_link *bpf_program__attach_freplace(const struct bpf_program *prog, > @@ -10705,7 +10710,7 @@ struct bpf_link *bpf_program__attach_freplace(const struct bpf_program *prog, > if (btf_id < 0) > return libbpf_err_ptr(btf_id); > > - return bpf_program__attach_fd(prog, target_fd, btf_id, "freplace"); > + return bpf_program__attach_fd(prog, target_fd, btf_id, "freplace", 0); > } else { > /* no target, so use raw_tracepoint_open for compatibility > * with old kernels > @@ -10760,6 +10765,12 @@ static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_l > return libbpf_get_error(*link); > } > > +struct bpf_link * > +bpf_program__attach_hid(const struct bpf_program *prog, int hid_fd, __u32 flags) > +{ > + return bpf_program__attach_fd(prog, hid_fd, 0, "hid", flags); > +} > + > struct bpf_link *bpf_program__attach(const struct bpf_program *prog) > { > struct bpf_link *link = NULL; > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index c1b0c2ef14d8..13dff4865da5 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -529,6 +529,8 @@ struct bpf_iter_attach_opts { > LIBBPF_API struct bpf_link * > bpf_program__attach_iter(const struct bpf_program *prog, > const struct bpf_iter_attach_opts *opts); > +LIBBPF_API struct bpf_link * > +bpf_program__attach_hid(const struct bpf_program *prog, int hid_fd, __u32 flags); > > /* > * Libbpf allows callers to adjust BPF programs before being loaded > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map > index df1b947792c8..cd8da9a8bf36 100644 > --- a/tools/lib/bpf/libbpf.map > +++ b/tools/lib/bpf/libbpf.map > @@ -442,6 +442,7 @@ LIBBPF_0.7.0 { > > LIBBPF_0.8.0 { > global: > + bpf_program__attach_hid; > libbpf_register_prog_handler; > libbpf_unregister_prog_handler; > } LIBBPF_0.7.0; > -- > 2.35.1 >
On Fri, Mar 18, 2022 at 9:17 AM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > Now that BPF can be compatible with HID, add the capability into HID. > drivers/hid/hid-bpf.c takes care of the glue between bpf and HID, and > hid-core can then inject any incoming event from the device into a BPF > program to filter/analyze it. So we only need this part for DEVICE EVENT? > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > changes in v3: > - squashed "only call hid_bpf_raw_event() if a ctx is available" > and "bpf: compute only the required buffer size for the device" > into this one > - ensure the ctx.size is properly bounded by allocated size > - s/link_attach/pre_link_attach/ > - s/array_detached/array_detach/ > - fix default switch case when doing nothing > - reworked hid_bpf_pre_link_attach() to avoid the switch > > changes in v2: > - split the series by bpf/libbpf/hid/selftests and samples > - addressed review comments from v1 > --- > drivers/hid/Makefile | 1 + > drivers/hid/hid-bpf.c | 174 +++++++++++++++++++++++++++++++++++++++++ > drivers/hid/hid-core.c | 24 +++++- > include/linux/hid.h | 11 +++ > 4 files changed, 207 insertions(+), 3 deletions(-) > create mode 100644 drivers/hid/hid-bpf.c > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index 6d3e630e81af..08d2d7619937 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -4,6 +4,7 @@ > # > hid-y := hid-core.o hid-input.o hid-quirks.o > hid-$(CONFIG_DEBUG_FS) += hid-debug.o > +hid-$(CONFIG_BPF) += hid-bpf.o > > obj-$(CONFIG_HID) += hid.o > obj-$(CONFIG_UHID) += uhid.o > diff --git a/drivers/hid/hid-bpf.c b/drivers/hid/hid-bpf.c > new file mode 100644 > index 000000000000..5060ebcb9979 > --- /dev/null > +++ b/drivers/hid/hid-bpf.c > @@ -0,0 +1,174 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * BPF in HID support for Linux > + * > + * Copyright (c) 2022 Benjamin Tissoires > + */ > + > +#include <linux/filter.h> > +#include <linux/mutex.h> > +#include <linux/slab.h> > + > +#include <uapi/linux/bpf_hid.h> > +#include <linux/hid.h> > + > +static int __hid_bpf_match_sysfs(struct device *dev, const void *data) > +{ > + struct kernfs_node *kn = dev->kobj.sd; > + struct kernfs_node *uevent_kn; > + > + uevent_kn = kernfs_find_and_get_ns(kn, "uevent", NULL); > + > + return uevent_kn == data; > +} > + > +static struct hid_device *hid_bpf_fd_to_hdev(int fd) > +{ > + struct device *dev; > + struct hid_device *hdev; > + struct fd f = fdget(fd); > + struct inode *inode; > + struct kernfs_node *node; > + > + if (!f.file) { > + hdev = ERR_PTR(-EBADF); > + goto out; > + } > + > + inode = file_inode(f.file); > + node = inode->i_private; > + > + dev = bus_find_device(&hid_bus_type, NULL, node, __hid_bpf_match_sysfs); > + > + if (dev) > + hdev = to_hid_device(dev); > + else > + hdev = ERR_PTR(-EINVAL); > + > + out: > + fdput(f); > + return hdev; > +} > + > +static int hid_bpf_pre_link_attach(struct hid_device *hdev, enum bpf_hid_attach_type type) > +{ > + int err = 0; > + unsigned int i, j, max_report_len = 0; > + unsigned int alloc_size = 0; > + > + if (type != BPF_HID_ATTACH_DEVICE_EVENT) > + return 0; > + > + /* hdev->bpf.device_data is already allocated, abort */ > + if (hdev->bpf.device_data) > + return 0; > + > + /* compute the maximum report length for this device */ > + for (i = 0; i < HID_REPORT_TYPES; i++) { > + struct hid_report_enum *report_enum = hdev->report_enum + i; > + > + for (j = 0; j < HID_MAX_IDS; j++) { > + struct hid_report *report = report_enum->report_id_hash[j]; > + > + if (report) > + max_report_len = max(max_report_len, hid_report_len(report)); > + } > + } > + > + /* > + * Give us a little bit of extra space and some predictability in the > + * buffer length we create. This way, we can tell users that they can > + * work on chunks of 64 bytes of memory without having the bpf verifier > + * scream at them. > + */ > + alloc_size = DIV_ROUND_UP(max_report_len, 64) * 64; > + > + hdev->bpf.device_data = kzalloc(alloc_size, GFP_KERNEL); > + if (!hdev->bpf.device_data) > + err = -ENOMEM; > + else > + hdev->bpf.allocated_data = alloc_size; > + > + return err; > +} > + > +static void hid_bpf_array_detach(struct hid_device *hdev, enum bpf_hid_attach_type type) > +{ > + switch (type) { > + case BPF_HID_ATTACH_DEVICE_EVENT: > + kfree(hdev->bpf.device_data); > + hdev->bpf.device_data = NULL; > + break; > + default: > + /* do nothing */ > + break; > + } > +} > + > +static int hid_bpf_run_progs(struct hid_device *hdev, struct hid_bpf_ctx_kern *ctx) > +{ > + enum bpf_hid_attach_type type; > + > + if (!ctx) > + return -EINVAL; > + > + switch (ctx->type) { > + case HID_BPF_DEVICE_EVENT: > + type = BPF_HID_ATTACH_DEVICE_EVENT; > + break; > + default: > + return -EINVAL; > + } > + > + if (!hdev->bpf.run_array[type]) > + return 0; > + > + return BPF_PROG_RUN_ARRAY(hdev->bpf.run_array[type], ctx, bpf_prog_run); > +} > + > +u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *data, int *size) > +{ > + int ret; > + struct hid_bpf_ctx_kern ctx = { > + .type = HID_BPF_DEVICE_EVENT, > + .hdev = hdev, > + .size = *size, > + .data = hdev->bpf.device_data, > + .allocated_size = hdev->bpf.allocated_data, > + }; > + > + if (bpf_hid_link_empty(&hdev->bpf, BPF_HID_ATTACH_DEVICE_EVENT)) > + return data; > + > + memset(ctx.data, 0, hdev->bpf.allocated_data); > + memcpy(ctx.data, data, *size); > + > + ret = hid_bpf_run_progs(hdev, &ctx); > + if (ret) > + return ERR_PTR(-EIO); > + > + if (!ctx.size || ctx.size > ctx.allocated_size) > + return ERR_PTR(-EINVAL); > + > + *size = ctx.size; > + > + return ctx.data; > +} > + > +int __init hid_bpf_module_init(void) > +{ > + struct bpf_hid_hooks hooks = { > + .hdev_from_fd = hid_bpf_fd_to_hdev, > + .pre_link_attach = hid_bpf_pre_link_attach, > + .array_detach = hid_bpf_array_detach, > + }; > + > + bpf_hid_set_hooks(&hooks); > + > + return 0; > +} > + > +void __exit hid_bpf_module_exit(void) > +{ > + bpf_hid_set_hooks(NULL); > +} > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index f1aed5bbd000..937fab7eb9c6 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1748,13 +1748,24 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, > struct hid_driver *hdrv; > unsigned int a; > u32 rsize, csize = size; > - u8 *cdata = data; > + u8 *cdata; > int ret = 0; > > + /* we pre-test if device_data is available here to cut the calls at the earliest */ > + if (hid->bpf.device_data) { > + data = hid_bpf_raw_event(hid, data, &size); > + if (IS_ERR(data)) { > + ret = PTR_ERR(data); > + goto out; > + } > + } > + > report = hid_get_report(report_enum, data); > if (!report) > goto out; > > + cdata = data; > + > if (report_enum->numbered) { > cdata++; > csize--; > @@ -2528,10 +2539,12 @@ int hid_add_device(struct hid_device *hdev) > > hid_debug_register(hdev, dev_name(&hdev->dev)); > ret = device_add(&hdev->dev); > - if (!ret) > + if (!ret) { > hdev->status |= HID_STAT_ADDED; > - else > + } else { > hid_debug_unregister(hdev); > + bpf_hid_exit(hdev); > + } > > return ret; > } > @@ -2567,6 +2580,7 @@ struct hid_device *hid_allocate_device(void) > spin_lock_init(&hdev->debug_list_lock); > sema_init(&hdev->driver_input_lock, 1); > mutex_init(&hdev->ll_open_lock); > + bpf_hid_init(hdev); > > return hdev; > } > @@ -2574,6 +2588,7 @@ EXPORT_SYMBOL_GPL(hid_allocate_device); > > static void hid_remove_device(struct hid_device *hdev) > { > + bpf_hid_exit(hdev); > if (hdev->status & HID_STAT_ADDED) { > device_del(&hdev->dev); > hid_debug_unregister(hdev); > @@ -2700,6 +2715,8 @@ static int __init hid_init(void) > > hid_debug_init(); > > + hid_bpf_module_init(); > + > return 0; > err_bus: > bus_unregister(&hid_bus_type); > @@ -2709,6 +2726,7 @@ static int __init hid_init(void) > > static void __exit hid_exit(void) > { > + hid_bpf_module_exit(); > hid_debug_exit(); > hidraw_exit(); > bus_unregister(&hid_bus_type); > diff --git a/include/linux/hid.h b/include/linux/hid.h > index 56f6f4ad45a7..8fd79011f461 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -27,6 +27,7 @@ > #include <linux/mutex.h> > #include <linux/power_supply.h> > #include <uapi/linux/hid.h> > +#include <uapi/linux/bpf_hid.h> > > /* > * We parse each description item into this structure. Short items data > @@ -1210,4 +1211,14 @@ do { \ > #define hid_dbg_once(hid, fmt, ...) \ > dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__) > > +#ifdef CONFIG_BPF > +u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *rd, int *size); > +int hid_bpf_module_init(void); > +void hid_bpf_module_exit(void); > +#else > +static inline u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *rd, int *size) { return rd; } > +static inline int hid_bpf_module_init(void) { return 0; } > +static inline void hid_bpf_module_exit(void) {} > +#endif > + > #endif > -- > 2.35.1 >
On Fri, Mar 18, 2022 at 9:17 AM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > Make use of BPF_HID_ATTACH_RDESC_FIXUP so we can trigger an rdesc fixup > in the bpf world. > > Whenever the program gets attached/detached, the device is reconnected > meaning that userspace will see it disappearing and reappearing with > the new report descriptor. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > changes in v3: > - ensure the ctx.size is properly bounded by allocated size > - s/link_attached/post_link_attach/ > - removed the switch statement with only one case > > changes in v2: > - split the series by bpf/libbpf/hid/selftests and samples > --- > drivers/hid/hid-bpf.c | 62 ++++++++++++++++++++++++++++++++++++++++++ > drivers/hid/hid-core.c | 3 +- > include/linux/hid.h | 6 ++++ > 3 files changed, 70 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-bpf.c b/drivers/hid/hid-bpf.c > index 5060ebcb9979..45c87ff47324 100644 > --- a/drivers/hid/hid-bpf.c > +++ b/drivers/hid/hid-bpf.c > @@ -50,6 +50,14 @@ static struct hid_device *hid_bpf_fd_to_hdev(int fd) > return hdev; > } > > +static int hid_reconnect(struct hid_device *hdev) > +{ > + if (!test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status)) > + return device_reprobe(&hdev->dev); > + > + return 0; > +} > + > static int hid_bpf_pre_link_attach(struct hid_device *hdev, enum bpf_hid_attach_type type) > { > int err = 0; > @@ -92,6 +100,12 @@ static int hid_bpf_pre_link_attach(struct hid_device *hdev, enum bpf_hid_attach_ > return err; > } > > +static void hid_bpf_post_link_attach(struct hid_device *hdev, enum bpf_hid_attach_type type) > +{ > + if (type == BPF_HID_ATTACH_RDESC_FIXUP) > + hid_reconnect(hdev); > +} > + > static void hid_bpf_array_detach(struct hid_device *hdev, enum bpf_hid_attach_type type) > { > switch (type) { > @@ -99,6 +113,9 @@ static void hid_bpf_array_detach(struct hid_device *hdev, enum bpf_hid_attach_ty > kfree(hdev->bpf.device_data); > hdev->bpf.device_data = NULL; > break; > + case BPF_HID_ATTACH_RDESC_FIXUP: > + hid_reconnect(hdev); > + break; > default: > /* do nothing */ > break; > @@ -116,6 +133,9 @@ static int hid_bpf_run_progs(struct hid_device *hdev, struct hid_bpf_ctx_kern *c > case HID_BPF_DEVICE_EVENT: > type = BPF_HID_ATTACH_DEVICE_EVENT; > break; > + case HID_BPF_RDESC_FIXUP: > + type = BPF_HID_ATTACH_RDESC_FIXUP; > + break; > default: > return -EINVAL; > } > @@ -155,11 +175,53 @@ u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *data, int *size) > return ctx.data; > } > > +u8 *hid_bpf_report_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size) > +{ > + int ret; > + struct hid_bpf_ctx_kern ctx = { > + .type = HID_BPF_RDESC_FIXUP, > + .hdev = hdev, > + .size = *size, > + }; > + > + if (bpf_hid_link_empty(&hdev->bpf, BPF_HID_ATTACH_RDESC_FIXUP)) Do we need to lock bpf_hid_mutex before calling bpf_hid_link_empty()? (or maybe we already did?) > + goto ignore_bpf; > + > + ctx.data = kmemdup(rdesc, HID_MAX_DESCRIPTOR_SIZE, GFP_KERNEL); > + if (!ctx.data) > + goto ignore_bpf; > + > + ctx.allocated_size = HID_MAX_DESCRIPTOR_SIZE; > + > + ret = hid_bpf_run_progs(hdev, &ctx); > + if (ret) > + goto ignore_bpf; > + > + if (ctx.size > ctx.allocated_size) > + goto ignore_bpf; > + > + *size = ctx.size; > + > + if (*size) { > + rdesc = krealloc(ctx.data, *size, GFP_KERNEL); > + } else { > + rdesc = NULL; > + kfree(ctx.data); > + } > + > + return rdesc; > + > + ignore_bpf: > + kfree(ctx.data); > + return kmemdup(rdesc, *size, GFP_KERNEL); > +} > + > int __init hid_bpf_module_init(void) > { > struct bpf_hid_hooks hooks = { > .hdev_from_fd = hid_bpf_fd_to_hdev, > .pre_link_attach = hid_bpf_pre_link_attach, > + .post_link_attach = hid_bpf_post_link_attach, > .array_detach = hid_bpf_array_detach, > }; > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 937fab7eb9c6..3182c39db006 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1213,7 +1213,8 @@ int hid_open_report(struct hid_device *device) > return -ENODEV; > size = device->dev_rsize; > > - buf = kmemdup(start, size, GFP_KERNEL); > + /* hid_bpf_report_fixup() ensures we work on a copy of rdesc */ > + buf = hid_bpf_report_fixup(device, start, &size); > if (buf == NULL) > return -ENOMEM; > > diff --git a/include/linux/hid.h b/include/linux/hid.h > index 8fd79011f461..66d949d10b78 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -1213,10 +1213,16 @@ do { \ > > #ifdef CONFIG_BPF > u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *rd, int *size); > +u8 *hid_bpf_report_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size); > int hid_bpf_module_init(void); > void hid_bpf_module_exit(void); > #else > static inline u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *rd, int *size) { return rd; } > +static inline u8 *hid_bpf_report_fixup(struct hid_device *hdev, u8 *rdesc, > + unsigned int *size) > +{ > + return kmemdup(rdesc, *size, GFP_KERNEL); > +} > static inline int hid_bpf_module_init(void) { return 0; } > static inline void hid_bpf_module_exit(void) {} > #endif > -- > 2.35.1 >
On Mon, Mar 21, 2022 at 9:20 AM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > On Fri, Mar 18, 2022 at 10:10 PM Song Liu <song@kernel.org> wrote: > > > > On Fri, Mar 18, 2022 at 9:17 AM Benjamin Tissoires > > <benjamin.tissoires@redhat.com> wrote: > > > > > > Make use of BPF_HID_ATTACH_RDESC_FIXUP so we can trigger an rdesc fixup > > > in the bpf world. > > > > > > Whenever the program gets attached/detached, the device is reconnected > > > meaning that userspace will see it disappearing and reappearing with > > > the new report descriptor. > > > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > > > > --- > > > > > > changes in v3: > > > - ensure the ctx.size is properly bounded by allocated size > > > - s/link_attached/post_link_attach/ > > > - removed the switch statement with only one case > > > > > > changes in v2: > > > - split the series by bpf/libbpf/hid/selftests and samples > > > --- > > > drivers/hid/hid-bpf.c | 62 ++++++++++++++++++++++++++++++++++++++++++ > > > drivers/hid/hid-core.c | 3 +- > > > include/linux/hid.h | 6 ++++ > > > 3 files changed, 70 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/hid/hid-bpf.c b/drivers/hid/hid-bpf.c > > > index 5060ebcb9979..45c87ff47324 100644 > > > --- a/drivers/hid/hid-bpf.c > > > +++ b/drivers/hid/hid-bpf.c > > > @@ -50,6 +50,14 @@ static struct hid_device *hid_bpf_fd_to_hdev(int fd) > > > return hdev; > > > } > > > > > > +static int hid_reconnect(struct hid_device *hdev) > > > +{ > > > + if (!test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status)) > > > + return device_reprobe(&hdev->dev); > > > + > > > + return 0; > > > +} > > > + > > > static int hid_bpf_pre_link_attach(struct hid_device *hdev, enum bpf_hid_attach_type type) > > > { > > > int err = 0; > > > @@ -92,6 +100,12 @@ static int hid_bpf_pre_link_attach(struct hid_device *hdev, enum bpf_hid_attach_ > > > return err; > > > } > > > > > > +static void hid_bpf_post_link_attach(struct hid_device *hdev, enum bpf_hid_attach_type type) > > > +{ > > > + if (type == BPF_HID_ATTACH_RDESC_FIXUP) > > > + hid_reconnect(hdev); > > > +} > > > + > > > static void hid_bpf_array_detach(struct hid_device *hdev, enum bpf_hid_attach_type type) > > > { > > > switch (type) { > > > @@ -99,6 +113,9 @@ static void hid_bpf_array_detach(struct hid_device *hdev, enum bpf_hid_attach_ty > > > kfree(hdev->bpf.device_data); > > > hdev->bpf.device_data = NULL; > > > break; > > > + case BPF_HID_ATTACH_RDESC_FIXUP: > > > + hid_reconnect(hdev); > > > + break; > > > default: > > > /* do nothing */ > > > break; > > > @@ -116,6 +133,9 @@ static int hid_bpf_run_progs(struct hid_device *hdev, struct hid_bpf_ctx_kern *c > > > case HID_BPF_DEVICE_EVENT: > > > type = BPF_HID_ATTACH_DEVICE_EVENT; > > > break; > > > + case HID_BPF_RDESC_FIXUP: > > > + type = BPF_HID_ATTACH_RDESC_FIXUP; > > > + break; > > > default: > > > return -EINVAL; > > > } > > > @@ -155,11 +175,53 @@ u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *data, int *size) > > > return ctx.data; > > > } > > > > > > +u8 *hid_bpf_report_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size) > > > +{ > > > + int ret; > > > + struct hid_bpf_ctx_kern ctx = { > > > + .type = HID_BPF_RDESC_FIXUP, > > > + .hdev = hdev, > > > + .size = *size, > > > + }; > > > + > > > + if (bpf_hid_link_empty(&hdev->bpf, BPF_HID_ATTACH_RDESC_FIXUP)) > > > > Do we need to lock bpf_hid_mutex before calling bpf_hid_link_empty()? > > (or maybe we > > already did?) > > The mutex is not locked before this call, indeed. > > However, bpf_hid_link_empty() is an inlined function that just calls > in the end list_empty(). Given that all the list heads are created > just once for the entire life of the HID device, I *think* this is > thread safe and does not require mutex locking. Hmm.. I guess you are right. > > (I might be wrong) > > So when first plugging in the device, if there is a fighting process > that attempts to add a program, if the program managed to insert > itself before we enter this code, then the list won't be empty and we > will execute BPF_PROG_RUN_ARRAY(), and if not, well, we ignore it and > wait for reconnect(). > > But now I am starting to wonder if I need to also protect > BPF_PROG_RUN_ARRAY() under bpf_hid_mutex... I think this is not necessary. Thanks, Song
Hi Benjamin, I tested this iteration of the set, and I faced couple of problems with it. 1) There were some conflicts as I could not figure out the correct kernel commit on which to apply the series on. I applied this on top of last weeks bpf-next (see below) with some local merge fixes. commit 2af7e566a8616c278e1d7287ce86cd3900bed943 (bpf-next/master, bpf-next/for-next) Author: Saeed Mahameed <saeedm@nvidia.com> Date: Tue Mar 22 10:22:24 2022 -0700 net/mlx5e: Fix build warning, detected write beyond size of field 2) hid_is_valid_access() causes some trouble and it rejects pretty much every BPF program which tries to use ctx->retval. This appears to be because prog->expected_attach_type is not populated, I had to apply below local tweak to overcome this problem: diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c index 30a62e8e0f0a..bf64411e6e9b 100644 --- a/kernel/bpf/hid.c +++ b/kernel/bpf/hid.c @@ -180,8 +180,7 @@ static bool hid_is_valid_access(int off, int size, case offsetof(struct hid_bpf_ctx, retval): if (size != size_default) return false; - return (prog->expected_attach_type == BPF_HID_USER_EVENT || - prog->expected_attach_type == BPF_HID_DRIVER_EVENT); + return true; default: if (size != size_default) return false; Proper fix would probably be to actually populate the expected_attach_type, but I could not figure out quickly where this should be done, or whether it is actually done on some other base commit. With those, for the whole series: Tested-by: Tero Kristo <tero.kristo@linux.intel.com> On 18/03/2022 18:15, Benjamin Tissoires wrote: > Hi, > > This is a followup of my v1 at [0] and v2 at [1]. > > The short summary of the previous cover letter and discussions is that > HID could benefit from BPF for the following use cases: > > - simple fixup of report descriptor: > benefits are faster development time and testing, with the produced > bpf program being shipped in the kernel directly (the shipping part > is *not* addressed here). > > - Universal Stylus Interface: > allows a user-space program to define its own kernel interface > > - Surface Dial: > somehow similar to the previous one except that userspace can decide > to change the shape of the exported device > > - firewall: > still partly missing there, there is not yet interception of hidraw > calls, but it's coming in a followup series, I promise > > - tracing: > well, tracing. > > > I think I addressed the comments from the previous version, but there are > a few things I'd like to note here: > > - I did not take the various rev-by and tested-by (thanks a lot for those) > because the uapi changed significantly in v3, so I am not very confident > in taking those rev-by blindly > > - I mentioned in my discussion with Song that I'll put a summary of the uapi > in the cover letter, but I ended up adding a (long) file in the Documentation > directory. So please maybe start by reading 17/17 to have an overview of > what I want to achieve > > - I added in the libbpf and bpf the new type BPF_HID_DRIVER_EVENT, even though > I don't have a user of it right now in the kernel. I wanted to have them in > the docs, but we might not want to have them ready here. > In terms of code, it just means that we can attach such programs types > but that they will never get triggered. > > Anyway, I have been mulling on this for the past 2 weeks, and I think that > maybe sharing this now is better than me just starring at the code over and > over. > > > Short summary of changes: > > v3: > === > > - squashed back together most of the libbpf and bpf changes into bigger > commits that give a better overview of the whole interactions > > - reworked the user API to not expose .data as a directly accessible field > from the context, but instead forces everyone to use hid_bpf_get_data (or > get/set_bits) > > - added BPF_HID_DRIVER_EVENT (see note above) > > - addressed the various nitpicks from v2 > > - added a big Documentation file (and so adding now the doc maintainers to the > long list of recipients) > > v2: > === > > - split the series by subsystem (bpf, HID, libbpf, selftests and > samples) > > - Added an extra patch at the beginning to not require CAP_NET_ADMIN for > BPF_PROG_TYPE_LIRC_MODE2 (please shout if this is wrong) > > - made the bpf context attached to HID program of dynamic size: > * the first 1 kB will be able to be addressed directly > * the rest can be retrieved through bpf_hid_{set|get}_data > (note that I am definitivey not happy with that API, because there > is part of it in bits and other in bytes. ouch) > > - added an extra patch to prevent non GPL HID bpf programs to be loaded > of type BPF_PROG_TYPE_HID > * same here, not really happy but I don't know where to put that check > in verifier.c > > - added a new flag BPF_F_INSERT_HEAD for BPF_LINK_CREATE syscall when in > used with HID program types. > * this flag is used for tracing, to be able to load a program before > any others that might already have been inserted and that might > change the data stream. > > Cheers, > Benjamin > > > > [0] https://lore.kernel.org/linux-input/20220224110828.2168231-1-benjamin.tissoires@redhat.com/T/#t > [1] https://lore.kernel.org/linux-input/20220304172852.274126-1-benjamin.tissoires@redhat.com/T/#t > > > Benjamin Tissoires (17): > bpf: add new is_sys_admin_prog_type() helper > bpf: introduce hid program type > bpf/verifier: prevent non GPL programs to be loaded against HID > libbpf: add HID program type and API > HID: hook up with bpf > HID: allow to change the report descriptor from an eBPF program > selftests/bpf: add tests for the HID-bpf initial implementation > selftests/bpf: add report descriptor fixup tests > selftests/bpf: Add a test for BPF_F_INSERT_HEAD > selftests/bpf: add test for user call of HID bpf programs > samples/bpf: add new hid_mouse example > bpf/hid: add more HID helpers > HID: bpf: implement hid_bpf_get|set_bits > HID: add implementation of bpf_hid_raw_request > selftests/bpf: add tests for hid_{get|set}_bits helpers > selftests/bpf: add tests for bpf_hid_hw_request > Documentation: add HID-BPF docs > > Documentation/hid/hid-bpf.rst | 444 +++++++++++ > Documentation/hid/index.rst | 1 + > drivers/hid/Makefile | 1 + > drivers/hid/hid-bpf.c | 328 ++++++++ > drivers/hid/hid-core.c | 34 +- > include/linux/bpf-hid.h | 127 +++ > include/linux/bpf_types.h | 4 + > include/linux/hid.h | 36 +- > include/uapi/linux/bpf.h | 67 ++ > include/uapi/linux/bpf_hid.h | 71 ++ > include/uapi/linux/hid.h | 10 + > kernel/bpf/Makefile | 3 + > kernel/bpf/btf.c | 1 + > kernel/bpf/hid.c | 728 +++++++++++++++++ > kernel/bpf/syscall.c | 27 +- > kernel/bpf/verifier.c | 7 + > samples/bpf/.gitignore | 1 + > samples/bpf/Makefile | 4 + > samples/bpf/hid_mouse_kern.c | 117 +++ > samples/bpf/hid_mouse_user.c | 129 +++ > tools/include/uapi/linux/bpf.h | 67 ++ > tools/lib/bpf/libbpf.c | 23 +- > tools/lib/bpf/libbpf.h | 2 + > tools/lib/bpf/libbpf.map | 1 + > tools/testing/selftests/bpf/config | 3 + > tools/testing/selftests/bpf/prog_tests/hid.c | 788 +++++++++++++++++++ > tools/testing/selftests/bpf/progs/hid.c | 205 +++++ > 27 files changed, 3204 insertions(+), 25 deletions(-) > create mode 100644 Documentation/hid/hid-bpf.rst > create mode 100644 drivers/hid/hid-bpf.c > create mode 100644 include/linux/bpf-hid.h > create mode 100644 include/uapi/linux/bpf_hid.h > create mode 100644 kernel/bpf/hid.c > create mode 100644 samples/bpf/hid_mouse_kern.c > create mode 100644 samples/bpf/hid_mouse_user.c > create mode 100644 tools/testing/selftests/bpf/prog_tests/hid.c > create mode 100644 tools/testing/selftests/bpf/progs/hid.c >
Hi Tero, On Tue, Mar 29, 2022 at 3:04 PM Tero Kristo <tero.kristo@linux.intel.com> wrote: > > Hi Benjamin, > > I tested this iteration of the set, and I faced couple of problems with it. > > 1) There were some conflicts as I could not figure out the correct > kernel commit on which to apply the series on. I applied this on top of > last weeks bpf-next (see below) with some local merge fixes. Right, there was a new conflict in bpf-next, but you managed it :) > > commit 2af7e566a8616c278e1d7287ce86cd3900bed943 (bpf-next/master, > bpf-next/for-next) > Author: Saeed Mahameed <saeedm@nvidia.com> > Date: Tue Mar 22 10:22:24 2022 -0700 > > net/mlx5e: Fix build warning, detected write beyond size of field > > 2) hid_is_valid_access() causes some trouble and it rejects pretty much > every BPF program which tries to use ctx->retval. This appears to be > because prog->expected_attach_type is not populated, I had to apply > below local tweak to overcome this problem: > > diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c > index 30a62e8e0f0a..bf64411e6e9b 100644 > --- a/kernel/bpf/hid.c > +++ b/kernel/bpf/hid.c > @@ -180,8 +180,7 @@ static bool hid_is_valid_access(int off, int size, > case offsetof(struct hid_bpf_ctx, retval): > if (size != size_default) > return false; > - return (prog->expected_attach_type == BPF_HID_USER_EVENT || > - prog->expected_attach_type == BPF_HID_DRIVER_EVENT); > + return true; > default: > if (size != size_default) > return false; > > Proper fix would probably be to actually populate the > expected_attach_type, but I could not figure out quickly where this > should be done, or whether it is actually done on some other base commit. Hmm, this is not what I would have expected. Anyway, "return true" would be a valid solution too, but... > > With those, for the whole series: > > Tested-by: Tero Kristo <tero.kristo@linux.intel.com> Thanks a lot. Unfortunately, if you saw the discussion with Alexei in patch 6/17, you'll see that there is a push toward a slightly different implementation. I had a meeting with Alexei, and a few other BPF folks yesterday, and they convinced me that this series is implementing a BPF feature the "old way", and that we should aim at having HID using standard BPF facilities instead of having HID messing up with bpf-core. This will be beneficial in the long term as we won't depend on BPF to be able to add new UAPI, being BPF calls or functions. I'll reply in more detail on 6/17. Cheers, Benjamin > > On 18/03/2022 18:15, Benjamin Tissoires wrote: > > Hi, > > > > This is a followup of my v1 at [0] and v2 at [1]. > > > > The short summary of the previous cover letter and discussions is that > > HID could benefit from BPF for the following use cases: > > > > - simple fixup of report descriptor: > > benefits are faster development time and testing, with the produced > > bpf program being shipped in the kernel directly (the shipping part > > is *not* addressed here). > > > > - Universal Stylus Interface: > > allows a user-space program to define its own kernel interface > > > > - Surface Dial: > > somehow similar to the previous one except that userspace can decide > > to change the shape of the exported device > > > > - firewall: > > still partly missing there, there is not yet interception of hidraw > > calls, but it's coming in a followup series, I promise > > > > - tracing: > > well, tracing. > > > > > > I think I addressed the comments from the previous version, but there are > > a few things I'd like to note here: > > > > - I did not take the various rev-by and tested-by (thanks a lot for those) > > because the uapi changed significantly in v3, so I am not very confident > > in taking those rev-by blindly > > > > - I mentioned in my discussion with Song that I'll put a summary of the uapi > > in the cover letter, but I ended up adding a (long) file in the Documentation > > directory. So please maybe start by reading 17/17 to have an overview of > > what I want to achieve > > > > - I added in the libbpf and bpf the new type BPF_HID_DRIVER_EVENT, even though > > I don't have a user of it right now in the kernel. I wanted to have them in > > the docs, but we might not want to have them ready here. > > In terms of code, it just means that we can attach such programs types > > but that they will never get triggered. > > > > Anyway, I have been mulling on this for the past 2 weeks, and I think that > > maybe sharing this now is better than me just starring at the code over and > > over. > > > > > > Short summary of changes: > > > > v3: > > === > > > > - squashed back together most of the libbpf and bpf changes into bigger > > commits that give a better overview of the whole interactions > > > > - reworked the user API to not expose .data as a directly accessible field > > from the context, but instead forces everyone to use hid_bpf_get_data (or > > get/set_bits) > > > > - added BPF_HID_DRIVER_EVENT (see note above) > > > > - addressed the various nitpicks from v2 > > > > - added a big Documentation file (and so adding now the doc maintainers to the > > long list of recipients) > > > > v2: > > === > > > > - split the series by subsystem (bpf, HID, libbpf, selftests and > > samples) > > > > - Added an extra patch at the beginning to not require CAP_NET_ADMIN for > > BPF_PROG_TYPE_LIRC_MODE2 (please shout if this is wrong) > > > > - made the bpf context attached to HID program of dynamic size: > > * the first 1 kB will be able to be addressed directly > > * the rest can be retrieved through bpf_hid_{set|get}_data > > (note that I am definitivey not happy with that API, because there > > is part of it in bits and other in bytes. ouch) > > > > - added an extra patch to prevent non GPL HID bpf programs to be loaded > > of type BPF_PROG_TYPE_HID > > * same here, not really happy but I don't know where to put that check > > in verifier.c > > > > - added a new flag BPF_F_INSERT_HEAD for BPF_LINK_CREATE syscall when in > > used with HID program types. > > * this flag is used for tracing, to be able to load a program before > > any others that might already have been inserted and that might > > change the data stream. > > > > Cheers, > > Benjamin > > > > > > > > [0] https://lore.kernel.org/linux-input/20220224110828.2168231-1-benjamin.tissoires@redhat.com/T/#t > > [1] https://lore.kernel.org/linux-input/20220304172852.274126-1-benjamin.tissoires@redhat.com/T/#t > > > > > > Benjamin Tissoires (17): > > bpf: add new is_sys_admin_prog_type() helper > > bpf: introduce hid program type > > bpf/verifier: prevent non GPL programs to be loaded against HID > > libbpf: add HID program type and API > > HID: hook up with bpf > > HID: allow to change the report descriptor from an eBPF program > > selftests/bpf: add tests for the HID-bpf initial implementation > > selftests/bpf: add report descriptor fixup tests > > selftests/bpf: Add a test for BPF_F_INSERT_HEAD > > selftests/bpf: add test for user call of HID bpf programs > > samples/bpf: add new hid_mouse example > > bpf/hid: add more HID helpers > > HID: bpf: implement hid_bpf_get|set_bits > > HID: add implementation of bpf_hid_raw_request > > selftests/bpf: add tests for hid_{get|set}_bits helpers > > selftests/bpf: add tests for bpf_hid_hw_request > > Documentation: add HID-BPF docs > > > > Documentation/hid/hid-bpf.rst | 444 +++++++++++ > > Documentation/hid/index.rst | 1 + > > drivers/hid/Makefile | 1 + > > drivers/hid/hid-bpf.c | 328 ++++++++ > > drivers/hid/hid-core.c | 34 +- > > include/linux/bpf-hid.h | 127 +++ > > include/linux/bpf_types.h | 4 + > > include/linux/hid.h | 36 +- > > include/uapi/linux/bpf.h | 67 ++ > > include/uapi/linux/bpf_hid.h | 71 ++ > > include/uapi/linux/hid.h | 10 + > > kernel/bpf/Makefile | 3 + > > kernel/bpf/btf.c | 1 + > > kernel/bpf/hid.c | 728 +++++++++++++++++ > > kernel/bpf/syscall.c | 27 +- > > kernel/bpf/verifier.c | 7 + > > samples/bpf/.gitignore | 1 + > > samples/bpf/Makefile | 4 + > > samples/bpf/hid_mouse_kern.c | 117 +++ > > samples/bpf/hid_mouse_user.c | 129 +++ > > tools/include/uapi/linux/bpf.h | 67 ++ > > tools/lib/bpf/libbpf.c | 23 +- > > tools/lib/bpf/libbpf.h | 2 + > > tools/lib/bpf/libbpf.map | 1 + > > tools/testing/selftests/bpf/config | 3 + > > tools/testing/selftests/bpf/prog_tests/hid.c | 788 +++++++++++++++++++ > > tools/testing/selftests/bpf/progs/hid.c | 205 +++++ > > 27 files changed, 3204 insertions(+), 25 deletions(-) > > create mode 100644 Documentation/hid/hid-bpf.rst > > create mode 100644 drivers/hid/hid-bpf.c > > create mode 100644 include/linux/bpf-hid.h > > create mode 100644 include/uapi/linux/bpf_hid.h > > create mode 100644 kernel/bpf/hid.c > > create mode 100644 samples/bpf/hid_mouse_kern.c > > create mode 100644 samples/bpf/hid_mouse_user.c > > create mode 100644 tools/testing/selftests/bpf/prog_tests/hid.c > > create mode 100644 tools/testing/selftests/bpf/progs/hid.c > > >
Hi, This is a followup of my v1 at [0] and v2 at [1]. The short summary of the previous cover letter and discussions is that HID could benefit from BPF for the following use cases: - simple fixup of report descriptor: benefits are faster development time and testing, with the produced bpf program being shipped in the kernel directly (the shipping part is *not* addressed here). - Universal Stylus Interface: allows a user-space program to define its own kernel interface - Surface Dial: somehow similar to the previous one except that userspace can decide to change the shape of the exported device - firewall: still partly missing there, there is not yet interception of hidraw calls, but it's coming in a followup series, I promise - tracing: well, tracing. I think I addressed the comments from the previous version, but there are a few things I'd like to note here: - I did not take the various rev-by and tested-by (thanks a lot for those) because the uapi changed significantly in v3, so I am not very confident in taking those rev-by blindly - I mentioned in my discussion with Song that I'll put a summary of the uapi in the cover letter, but I ended up adding a (long) file in the Documentation directory. So please maybe start by reading 17/17 to have an overview of what I want to achieve - I added in the libbpf and bpf the new type BPF_HID_DRIVER_EVENT, even though I don't have a user of it right now in the kernel. I wanted to have them in the docs, but we might not want to have them ready here. In terms of code, it just means that we can attach such programs types but that they will never get triggered. Anyway, I have been mulling on this for the past 2 weeks, and I think that maybe sharing this now is better than me just starring at the code over and over. Short summary of changes: v3: === - squashed back together most of the libbpf and bpf changes into bigger commits that give a better overview of the whole interactions - reworked the user API to not expose .data as a directly accessible field from the context, but instead forces everyone to use hid_bpf_get_data (or get/set_bits) - added BPF_HID_DRIVER_EVENT (see note above) - addressed the various nitpicks from v2 - added a big Documentation file (and so adding now the doc maintainers to the long list of recipients) v2: === - split the series by subsystem (bpf, HID, libbpf, selftests and samples) - Added an extra patch at the beginning to not require CAP_NET_ADMIN for BPF_PROG_TYPE_LIRC_MODE2 (please shout if this is wrong) - made the bpf context attached to HID program of dynamic size: * the first 1 kB will be able to be addressed directly * the rest can be retrieved through bpf_hid_{set|get}_data (note that I am definitivey not happy with that API, because there is part of it in bits and other in bytes. ouch) - added an extra patch to prevent non GPL HID bpf programs to be loaded of type BPF_PROG_TYPE_HID * same here, not really happy but I don't know where to put that check in verifier.c - added a new flag BPF_F_INSERT_HEAD for BPF_LINK_CREATE syscall when in used with HID program types. * this flag is used for tracing, to be able to load a program before any others that might already have been inserted and that might change the data stream. Cheers, Benjamin [0] https://lore.kernel.org/linux-input/20220224110828.2168231-1-benjamin.tissoires@redhat.com/T/#t [1] https://lore.kernel.org/linux-input/20220304172852.274126-1-benjamin.tissoires@redhat.com/T/#t Benjamin Tissoires (17): bpf: add new is_sys_admin_prog_type() helper bpf: introduce hid program type bpf/verifier: prevent non GPL programs to be loaded against HID libbpf: add HID program type and API HID: hook up with bpf HID: allow to change the report descriptor from an eBPF program selftests/bpf: add tests for the HID-bpf initial implementation selftests/bpf: add report descriptor fixup tests selftests/bpf: Add a test for BPF_F_INSERT_HEAD selftests/bpf: add test for user call of HID bpf programs samples/bpf: add new hid_mouse example bpf/hid: add more HID helpers HID: bpf: implement hid_bpf_get|set_bits HID: add implementation of bpf_hid_raw_request selftests/bpf: add tests for hid_{get|set}_bits helpers selftests/bpf: add tests for bpf_hid_hw_request Documentation: add HID-BPF docs Documentation/hid/hid-bpf.rst | 444 +++++++++++ Documentation/hid/index.rst | 1 + drivers/hid/Makefile | 1 + drivers/hid/hid-bpf.c | 328 ++++++++ drivers/hid/hid-core.c | 34 +- include/linux/bpf-hid.h | 127 +++ include/linux/bpf_types.h | 4 + include/linux/hid.h | 36 +- include/uapi/linux/bpf.h | 67 ++ include/uapi/linux/bpf_hid.h | 71 ++ include/uapi/linux/hid.h | 10 + kernel/bpf/Makefile | 3 + kernel/bpf/btf.c | 1 + kernel/bpf/hid.c | 728 +++++++++++++++++ kernel/bpf/syscall.c | 27 +- kernel/bpf/verifier.c | 7 + samples/bpf/.gitignore | 1 + samples/bpf/Makefile | 4 + samples/bpf/hid_mouse_kern.c | 117 +++ samples/bpf/hid_mouse_user.c | 129 +++ tools/include/uapi/linux/bpf.h | 67 ++ tools/lib/bpf/libbpf.c | 23 +- tools/lib/bpf/libbpf.h | 2 + tools/lib/bpf/libbpf.map | 1 + tools/testing/selftests/bpf/config | 3 + tools/testing/selftests/bpf/prog_tests/hid.c | 788 +++++++++++++++++++ tools/testing/selftests/bpf/progs/hid.c | 205 +++++ 27 files changed, 3204 insertions(+), 25 deletions(-) create mode 100644 Documentation/hid/hid-bpf.rst create mode 100644 drivers/hid/hid-bpf.c create mode 100644 include/linux/bpf-hid.h create mode 100644 include/uapi/linux/bpf_hid.h create mode 100644 kernel/bpf/hid.c create mode 100644 samples/bpf/hid_mouse_kern.c create mode 100644 samples/bpf/hid_mouse_user.c create mode 100644 tools/testing/selftests/bpf/prog_tests/hid.c create mode 100644 tools/testing/selftests/bpf/progs/hid.c