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.
(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...
Cheers, Benjamin
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