2022-02-24 11:39:19

by Benjamin Tissoires

Subject: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

Hi there,

This series introduces support of eBPF for HID devices.

I have several use cases where eBPF could be interesting for those
input devices:

- simple fixup of report descriptor:

In the HID tree, we have half of the drivers that are "simple" and
that just fix one key or one byte in the report descriptor.
Currently, for users of such devices, the process of fixing them
is long and painful.
With eBPF, we could externalize those fixups in one external repo,
ship various CoRe bpf programs and have those programs loaded at boot
time without having to install a new kernel (and wait 6 months for the
fix to land in the distro kernel)

- Universal Stylus Interface (or any other new fancy feature that
requires a new kernel API)

See [0].
Basically, USI pens are requiring a new kernel API because there are
some channels of communication our HID and input stack are not capable
of. Instead of using hidraw or creating new sysfs or ioctls, we can rely
on eBPF to have the kernel API controlled by the consumer and to not
impact the performances by waking up userspace every time there is an
event.

- Surface Dial

This device is a "puck" from Microsoft, basically a rotary dial with a
push button. The kernel already exports it as such but doesn't handle
the haptic feedback we can get out of it.
Furthermore, that device is not recognized by userspace and so it's a
nice paperwight in the end.

With eBPF, we can morph that device into a mouse, and convert the dial
events into wheel events. Also, we can set/unset the haptic feedback
from userspace. The convenient part of BPF makes it that the kernel
doesn't make any choice that would need to be reverted because that
specific userspace doesn't handle it properly or because that other
one expects it to be different.

- 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)
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.

- tracing

The last usage I have in mind is tracing events and all the fun we can
do we BPF to summarize and analyze events.
Right now, tracing relies on hidraw. It works well except for a couple
of issues:
1. if the driver doesn't export a hidraw node, we can't trace anything
(eBPF will be a "god-mode" there, so it might raise some eyebrows)
2. hidraw doesn't catch the other process requests to the device, which
means that we have cases where we need to add printks to the kernel
to understand what is happening.


With that long introduction, here is the v1 of the support of eBPF in
HID.

I have targeted bpf-next here because the parts that will have the most
conflicts are in bpf. There might be a trivial minor conflict in
include/linux/hid.h with an other series I have pending[1].

I am relatively new to bpf, so having some feedback would be most very
welcome.

A couple of notes though:

- The series is missing a SEC("hid/driver_event") which would allow to
intercept incoming requests to the device from anybody. I left it
outside because it's not critical to have it from day one (we are more
interested right now by the USI case above)

- I am still wondering how to integrate the tracing part:
right now, if a bpf program is loaded before we start the tracer, we
will see *modified* events in the tracer. However, it might be
interesting to decide to see either unmodified (raw events from the
device) or modified events.
I think a flag might be able to solve that. The flag will control
whether we add the new program at the beginning of the list or at the
tail, but I am not sure if this is common practice in eBPF or if
there is a better way.

Cheers,
Benjamin


[0] https://lore.kernel.org/linux-input/[email protected]/
[1] https://lore.kernel.org/linux-input/[email protected]/


Benjamin Tissoires (6):
HID: initial BPF implementation
HID: bpf: allow to change the report descriptor from an eBPF program
HID: bpf: add hid_{get|set}_data helpers
HID: bpf: add new BPF type to trigger commands from userspace
HID: bpf: tests: rely on uhid event to know if a test device is ready
HID: bpf: add bpf_hid_raw_request helper function

drivers/hid/Makefile | 1 +
drivers/hid/hid-bpf.c | 327 +++++++++
drivers/hid/hid-core.c | 31 +-
include/linux/bpf-hid.h | 98 +++
include/linux/bpf_types.h | 4 +
include/linux/hid.h | 25 +
include/uapi/linux/bpf.h | 33 +
include/uapi/linux/bpf_hid.h | 56 ++
kernel/bpf/Makefile | 3 +
kernel/bpf/hid.c | 653 ++++++++++++++++++
kernel/bpf/syscall.c | 12 +
samples/bpf/.gitignore | 1 +
samples/bpf/Makefile | 4 +
samples/bpf/hid_mouse_kern.c | 91 +++
samples/bpf/hid_mouse_user.c | 129 ++++
tools/include/uapi/linux/bpf.h | 33 +
tools/lib/bpf/libbpf.c | 9 +
tools/lib/bpf/libbpf.h | 2 +
tools/lib/bpf/libbpf.map | 1 +
tools/testing/selftests/bpf/prog_tests/hid.c | 685 +++++++++++++++++++
tools/testing/selftests/bpf/progs/hid.c | 149 ++++
21 files changed, 2339 insertions(+), 8 deletions(-)
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

--
2.35.1

2022-02-24 11:57:31

by Benjamin Tissoires

Subject: [PATCH bpf-next v1 3/6] HID: bpf: add hid_{get|set}_data helpers

When we process an incoming HID report, it is common to have to account
for fields that are not aligned in the report. HID is using 2 helpers
hid_field_extract() and implement() to pick up any data at any offset
within the report.

Export those 2 helpers in BPF programs so users can also rely on them.
The second net worth advantage of those helpers is that now we can
fetch data anywhere in the report without knowing at compile time the
location of it. The boundary checks are done in hid-bpf.c, to prevent
a memory leak.

Signed-off-by: Benjamin Tissoires <[email protected]>
---
drivers/hid/hid-bpf.c | 22 +++++++
drivers/hid/hid-core.c | 4 +-
include/linux/bpf-hid.h | 2 +
include/linux/hid.h | 2 +
include/uapi/linux/bpf.h | 16 +++++
kernel/bpf/hid.c | 68 ++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 16 +++++
tools/testing/selftests/bpf/prog_tests/hid.c | 59 +++++++++++++++++
tools/testing/selftests/bpf/progs/hid.c | 14 ++++
9 files changed, 201 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-bpf.c b/drivers/hid/hid-bpf.c
index 2d54c87cda1a..d775bda9d28d 100644
--- a/drivers/hid/hid-bpf.c
+++ b/drivers/hid/hid-bpf.c
@@ -118,6 +118,26 @@ static void hid_bpf_array_detached(struct hid_device *hdev, enum bpf_hid_attach_
}
}

+int hid_bpf_get_data(struct hid_device *hdev, u8 *buf, u64 offset, u8 n)
+{
+ if (n > 32 ||
+ ((offset + n) >> 3) >= HID_BPF_MAX_BUFFER_SIZE)
+ return 0;
+
+ return hid_field_extract(hdev, buf, offset, n);
+}
+
+int hid_bpf_set_data(struct hid_device *hdev, u8 *buf, u64 offset, u8 n, u32 data)
+{
+ if (n > 32 ||
+ ((offset + n) >> 3) >= HID_BPF_MAX_BUFFER_SIZE)
+ return -EINVAL;
+
+ implement(hdev, buf, offset, n, data);
+
+ return 0;
+}
+
static int hid_bpf_run_progs(struct hid_device *hdev, enum bpf_hid_attach_type type,
struct hid_bpf_ctx *ctx, u8 *data, int size)
{
@@ -229,6 +249,8 @@ int __init hid_bpf_module_init(void)
.link_attach = hid_bpf_link_attach,
.link_attached = hid_bpf_link_attached,
.array_detached = hid_bpf_array_detached,
+ .hid_get_data = hid_bpf_get_data,
+ .hid_set_data = hid_bpf_set_data,
};

bpf_hid_set_hooks(&hooks);
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 0eb8189faaee..d3f4499ee4cd 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1416,8 +1416,8 @@ static void __implement(u8 *report, unsigned offset, int n, u32 value)
}
}

-static void implement(const struct hid_device *hid, u8 *report,
- unsigned offset, unsigned n, u32 value)
+void implement(const struct hid_device *hid, u8 *report, unsigned int offset, unsigned int n,
+ u32 value)
{
if (unlikely(n > 32)) {
hid_warn(hid, "%s() called with n (%d) > 32! (%s)\n",
diff --git a/include/linux/bpf-hid.h b/include/linux/bpf-hid.h
index 377012a019da..07cbd5cf595c 100644
--- a/include/linux/bpf-hid.h
+++ b/include/linux/bpf-hid.h
@@ -72,6 +72,8 @@ struct bpf_hid_hooks {
int (*link_attach)(struct hid_device *hdev, enum bpf_hid_attach_type type);
void (*link_attached)(struct hid_device *hdev, enum bpf_hid_attach_type type);
void (*array_detached)(struct hid_device *hdev, enum bpf_hid_attach_type type);
+ int (*hid_get_data)(struct hid_device *hdev, u8 *buf, u64 offset, u8 size);
+ int (*hid_set_data)(struct hid_device *hdev, u8 *buf, u64 offset, u8 size, u32 data);
};

#ifdef CONFIG_BPF
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 66d949d10b78..7454e844324c 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -944,6 +944,8 @@ bool hid_compare_device_paths(struct hid_device *hdev_a,
s32 hid_snto32(__u32 value, unsigned n);
__u32 hid_field_extract(const struct hid_device *hid, __u8 *report,
unsigned offset, unsigned n);
+void implement(const struct hid_device *hid, u8 *report, unsigned int offset, unsigned int n,
+ u32 value);

#ifdef CONFIG_PM
int hid_driver_suspend(struct hid_device *hdev, pm_message_t state);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a7a8d9cfcf24..0571d9b954c9 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5090,6 +5090,20 @@ union bpf_attr {
* Return
* 0 on success, or a negative error in case of failure. On error
* *dst* buffer is zeroed out.
+ *
+ * u32 bpf_hid_get_data(void *ctx, u64 offset, u8 n)
+ * Description
+ * Get the data of size n at the given offset in the
+ * ctx->event.data field
+ * Return
+ * The value at offset. In case of error: 0.
+ *
+ * int bpf_hid_set_data(void *ctx, u64 offset, u8 n, u32 data)
+ * Description
+ * Set the data of size n at the given offset in the
+ * ctx->event.data field
+ * Return
+ * 0 on success, a negative error on failure.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -5284,6 +5298,8 @@ union bpf_attr {
FN(xdp_load_bytes), \
FN(xdp_store_bytes), \
FN(copy_from_user_task), \
+ FN(hid_get_data), \
+ FN(hid_set_data), \
/* */

/* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c
index 47cb0580b14a..9eb7bd6ac6c8 100644
--- a/kernel/bpf/hid.c
+++ b/kernel/bpf/hid.c
@@ -37,10 +37,78 @@ void bpf_hid_set_hooks(struct bpf_hid_hooks *hooks)
}
EXPORT_SYMBOL_GPL(bpf_hid_set_hooks);

+BPF_CALL_3(bpf_hid_get_data, void*, ctx, u64, offset, u8, n)
+{
+ struct hid_bpf_ctx *bpf_ctx = ctx;
+ u8 *buf;
+
+ if (!hid_hooks.hid_get_data)
+ return -EOPNOTSUPP;
+
+ switch (bpf_ctx->type) {
+ case HID_BPF_DEVICE_EVENT:
+ buf = bpf_ctx->u.device.data;
+ break;
+ case HID_BPF_RDESC_FIXUP:
+ buf = bpf_ctx->u.rdesc.data;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return hid_hooks.hid_get_data(bpf_ctx->hdev, buf, offset, n);
+}
+
+static const struct bpf_func_proto bpf_hid_get_data_proto = {
+ .func = bpf_hid_get_data,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+};
+
+BPF_CALL_4(bpf_hid_set_data, void*, ctx, u64, offset, u8, n, u32, data)
+{
+ struct hid_bpf_ctx *bpf_ctx = ctx;
+ u8 *buf;
+
+ if (!hid_hooks.hid_set_data)
+ return -EOPNOTSUPP;
+
+ switch (bpf_ctx->type) {
+ case HID_BPF_DEVICE_EVENT:
+ buf = bpf_ctx->u.device.data;
+ break;
+ case HID_BPF_RDESC_FIXUP:
+ buf = bpf_ctx->u.rdesc.data;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ hid_hooks.hid_set_data(bpf_ctx->hdev, buf, offset, n, data);
+ return 0;
+}
+
+static const struct bpf_func_proto bpf_hid_set_data_proto = {
+ .func = bpf_hid_set_data,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+ .arg4_type = ARG_ANYTHING,
+};
+
static const struct bpf_func_proto *
hid_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
switch (func_id) {
+ case BPF_FUNC_hid_get_data:
+ return &bpf_hid_get_data_proto;
+ case BPF_FUNC_hid_set_data:
+ return &bpf_hid_set_data_proto;
default:
return bpf_base_func_proto(func_id);
}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a7a8d9cfcf24..0571d9b954c9 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5090,6 +5090,20 @@ union bpf_attr {
* Return
* 0 on success, or a negative error in case of failure. On error
* *dst* buffer is zeroed out.
+ *
+ * u32 bpf_hid_get_data(void *ctx, u64 offset, u8 n)
+ * Description
+ * Get the data of size n at the given offset in the
+ * ctx->event.data field
+ * Return
+ * The value at offset. In case of error: 0.
+ *
+ * int bpf_hid_set_data(void *ctx, u64 offset, u8 n, u32 data)
+ * Description
+ * Set the data of size n at the given offset in the
+ * ctx->event.data field
+ * Return
+ * 0 on success, a negative error on failure.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -5284,6 +5298,8 @@ union bpf_attr {
FN(xdp_load_bytes), \
FN(xdp_store_bytes), \
FN(copy_from_user_task), \
+ FN(hid_get_data), \
+ FN(hid_set_data), \
/* */

/* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index dccbbcaa69e5..7d4f740a0a08 100644
--- a/tools/testing/selftests/bpf/prog_tests/hid.c
+++ b/tools/testing/selftests/bpf/prog_tests/hid.c
@@ -280,6 +280,62 @@ static int test_hid_raw_event(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
return ret;
}

+/*
+ * Attach hid_set_get_data to the given uhid device,
+ * retrieve and open the matching hidraw node,
+ * inject one event in the uhid device,
+ * check that the program makes correct use of bpf_hid_{set|get}_data.
+ */
+static int test_hid_set_get_data(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
+{
+ int err, hidraw_ino, hidraw_fd = -1;
+ char hidraw_path[64] = {0};
+ u8 buf[10] = {0};
+ int ret = -1;
+
+ /* attach hid_set_get_data program */
+ hid_skel->links.hid_set_get_data =
+ bpf_program__attach_hid(hid_skel->progs.hid_set_get_data, sysfs_fd);
+ if (!ASSERT_OK_PTR(hid_skel->links.hid_set_get_data,
+ "attach_hid(hid_set_get_data)"))
+ return PTR_ERR(hid_skel->links.hid_set_get_data);
+
+ hidraw_ino = get_hidraw(hid_skel->links.hid_set_get_data);
+ if (!ASSERT_GE(hidraw_ino, 0, "get_hidraw"))
+ goto cleanup;
+
+ /* open hidraw node to check the other side of the pipe */
+ sprintf(hidraw_path, "/dev/hidraw%d", hidraw_ino);
+ hidraw_fd = open(hidraw_path, O_RDWR | O_NONBLOCK);
+
+ if (!ASSERT_GE(hidraw_fd, 0, "open_hidraw"))
+ goto cleanup;
+
+ /* inject one event */
+ buf[0] = 1;
+ buf[1] = 42;
+ send_event(uhid_fd, buf, 4);
+
+ /* read the data from hidraw */
+ memset(buf, 0, sizeof(buf));
+ err = read(hidraw_fd, buf, sizeof(buf));
+ if (!ASSERT_EQ(err, 4, "read_hidraw"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[2], (42 >> 2), "hid_set_get_data"))
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ if (hidraw_fd >= 0)
+ close(hidraw_fd);
+
+ hid__detach(hid_skel);
+
+ return ret;
+}
+
/*
* Attach hid_rdesc_fixup to the given uhid device,
* retrieve and open the matching hidraw node,
@@ -378,6 +434,9 @@ void serial_test_hid_bpf(void)
err = test_hid_raw_event(hid_skel, uhid_fd, sysfs_fd);
ASSERT_OK(err, "hid");

+ err = test_hid_set_get_data(hid_skel, uhid_fd, sysfs_fd);
+ ASSERT_OK(err, "hid_set_get_data");
+
err = test_rdesc_fixup(hid_skel, uhid_fd, sysfs_fd);
ASSERT_OK(err, "hid_rdesc_fixup");

diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
index f7a64c637782..01d9c556a3a1 100644
--- a/tools/testing/selftests/bpf/progs/hid.c
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -66,3 +66,17 @@ int hid_rdesc_fixup(struct hid_bpf_ctx *ctx)

return 0;
}
+
+SEC("hid/device_event")
+int hid_set_get_data(struct hid_bpf_ctx *ctx)
+{
+ __u32 x;
+
+ /* extract data at bit offset 10 of size 4 (half a byte) */
+ x = bpf_hid_get_data(ctx, 10, 4);
+
+ /* reinject it */
+ bpf_hid_set_data(ctx, 16, 4, x);
+
+ return 0;
+}
--
2.35.1

2022-02-24 12:22:43

by Benjamin Tissoires

Subject: [PATCH bpf-next v1 1/6] HID: initial BPF implementation

HID is a protocol that could benefit from using BPF too.

This patch implements a net-like use of BPF capability for HID.
Any incoming report coming from the device gets injected into a series
of BPF programs that can modify it or even discard it by setting the
size in the context to 0.

The kernel/bpf implementation is based on net-namespace.c, with only
the bpf_link part kept, there is no real points in keeping the
bpf_prog_{attach|detach} API.

The implementation is split into 2 parts:
- the kernel/bpf part which isn't aware of the HID usage, but takes care
of handling the BPF links
- the drivers/hid/hid-bpf.c part which knows about HID

Note that HID can be compiled in as a module, and so the functions that
kernel/bpf/hid.c needs to call in hid.ko are exported in struct hid_hooks.

Signed-off-by: Benjamin Tissoires <[email protected]>
---
drivers/hid/Makefile | 1 +
drivers/hid/hid-bpf.c | 176 ++++++++
drivers/hid/hid-core.c | 21 +-
include/linux/bpf-hid.h | 87 ++++
include/linux/bpf_types.h | 4 +
include/linux/hid.h | 16 +
include/uapi/linux/bpf.h | 7 +
include/uapi/linux/bpf_hid.h | 39 ++
kernel/bpf/Makefile | 3 +
kernel/bpf/hid.c | 437 +++++++++++++++++++
kernel/bpf/syscall.c | 8 +
samples/bpf/.gitignore | 1 +
samples/bpf/Makefile | 4 +
samples/bpf/hid_mouse_kern.c | 66 +++
samples/bpf/hid_mouse_user.c | 129 ++++++
tools/include/uapi/linux/bpf.h | 7 +
tools/lib/bpf/libbpf.c | 7 +
tools/lib/bpf/libbpf.h | 2 +
tools/lib/bpf/libbpf.map | 1 +
tools/testing/selftests/bpf/prog_tests/hid.c | 318 ++++++++++++++
tools/testing/selftests/bpf/progs/hid.c | 20 +
21 files changed, 1351 insertions(+), 3 deletions(-)
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

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..6c8445820944
--- /dev/null
+++ b/drivers/hid/hid-bpf.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * BPF in HID support for Linux
+ *
+ * Copyright (c) 2021 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 struct hid_bpf_ctx *hid_bpf_allocate_ctx(struct hid_device *hdev)
+{
+ struct hid_bpf_ctx *ctx;
+
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return ERR_PTR(-ENOMEM);
+
+ ctx->hdev = hdev;
+
+ return ctx;
+}
+
+static int hid_bpf_link_attach(struct hid_device *hdev, enum bpf_hid_attach_type type)
+{
+ int err = 0;
+
+ switch (type) {
+ case BPF_HID_ATTACH_DEVICE_EVENT:
+ if (!hdev->bpf.ctx) {
+ hdev->bpf.ctx = hid_bpf_allocate_ctx(hdev);
+ if (IS_ERR(hdev->bpf.ctx)) {
+ err = PTR_ERR(hdev->bpf.ctx);
+ hdev->bpf.ctx = NULL;
+ }
+ }
+ break;
+ default:
+ /* do nothing */
+ }
+
+ return err;
+}
+
+static void hid_bpf_array_detached(struct hid_device *hdev, enum bpf_hid_attach_type type)
+{
+ switch (type) {
+ case BPF_HID_ATTACH_DEVICE_EVENT:
+ kfree(hdev->bpf.ctx);
+ hdev->bpf.ctx = NULL;
+ break;
+ default:
+ /* do nothing */
+ }
+}
+
+static int hid_bpf_run_progs(struct hid_device *hdev, enum bpf_hid_attach_type type,
+ struct hid_bpf_ctx *ctx, u8 *data, int size)
+{
+ enum hid_bpf_event event = HID_BPF_UNDEF;
+
+ if (type < 0 || !ctx)
+ return -EINVAL;
+
+ switch (type) {
+ case BPF_HID_ATTACH_DEVICE_EVENT:
+ event = HID_BPF_DEVICE_EVENT;
+ if (size > sizeof(ctx->u.device.data))
+ return -E2BIG;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (!hdev->bpf.run_array[type])
+ return 0;
+
+ memset(ctx, 0, sizeof(*ctx));
+ ctx->hdev = hdev;
+ ctx->type = event;
+
+ if (size && data) {
+ switch (event) {
+ case HID_BPF_DEVICE_EVENT:
+ memcpy(ctx->u.device.data, data, size);
+ ctx->u.device.size = size;
+ break;
+ default:
+ /* do nothing */
+ }
+ }
+
+ BPF_PROG_RUN_ARRAY(hdev->bpf.run_array[type], ctx, bpf_prog_run);
+
+ return 0;
+}
+
+u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *data, int *size)
+{
+ int ret;
+
+ if (bpf_hid_link_empty(&hdev->bpf, BPF_HID_ATTACH_DEVICE_EVENT))
+ return data;
+
+ ret = hid_bpf_run_progs(hdev, BPF_HID_ATTACH_DEVICE_EVENT,
+ hdev->bpf.ctx, data, *size);
+ if (ret)
+ return data;
+
+ if (!hdev->bpf.ctx->u.device.size)
+ return ERR_PTR(-EINVAL);
+
+ *size = hdev->bpf.ctx->u.device.size;
+
+ return hdev->bpf.ctx->u.device.data;
+}
+
+int __init hid_bpf_module_init(void)
+{
+ struct bpf_hid_hooks hooks = {
+ .hdev_from_fd = hid_bpf_fd_to_hdev,
+ .link_attach = hid_bpf_link_attach,
+ .array_detached = hid_bpf_array_detached,
+ };
+
+ 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..a80bffe6ce4a 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1748,13 +1748,21 @@ 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;

+ 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 +2536,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 +2577,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 +2585,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 +2712,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 +2723,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/bpf-hid.h b/include/linux/bpf-hid.h
new file mode 100644
index 000000000000..363fb6a4923f
--- /dev/null
+++ b/include/linux/bpf-hid.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _BPF_HID_H
+#define _BPF_HID_H
+
+#include <linux/mutex.h>
+#include <uapi/linux/bpf.h>
+#include <uapi/linux/bpf_hid.h>
+#include <linux/list.h>
+
+struct bpf_prog;
+struct bpf_prog_array;
+struct hid_device;
+
+enum bpf_hid_attach_type {
+ BPF_HID_ATTACH_INVALID = -1,
+ BPF_HID_ATTACH_DEVICE_EVENT = 0,
+ MAX_BPF_HID_ATTACH_TYPE
+};
+
+struct bpf_hid {
+ struct hid_bpf_ctx *ctx;
+
+ /* Array of programs to run compiled from links */
+ struct bpf_prog_array __rcu *run_array[MAX_BPF_HID_ATTACH_TYPE];
+ struct list_head links[MAX_BPF_HID_ATTACH_TYPE];
+};
+
+static inline enum bpf_hid_attach_type
+to_bpf_hid_attach_type(enum bpf_attach_type attach_type)
+{
+ switch (attach_type) {
+ case BPF_HID_DEVICE_EVENT:
+ return BPF_HID_ATTACH_DEVICE_EVENT;
+ default:
+ return BPF_HID_ATTACH_INVALID;
+ }
+}
+
+union bpf_attr;
+struct bpf_prog;
+
+#if IS_ENABLED(CONFIG_HID)
+int bpf_hid_prog_query(const union bpf_attr *attr,
+ union bpf_attr __user *uattr);
+int bpf_hid_link_create(const union bpf_attr *attr,
+ struct bpf_prog *prog);
+#else
+static inline int bpf_hid_prog_query(const union bpf_attr *attr,
+ union bpf_attr __user *uattr)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int bpf_hid_link_create(const union bpf_attr *attr,
+ struct bpf_prog *prog)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
+static inline bool bpf_hid_link_empty(struct bpf_hid *bpf,
+ enum bpf_hid_attach_type type)
+{
+ return list_empty(&bpf->links[type]);
+}
+
+struct bpf_hid_hooks {
+ struct hid_device *(*hdev_from_fd)(int fd);
+ int (*link_attach)(struct hid_device *hdev, enum bpf_hid_attach_type type);
+ void (*array_detached)(struct hid_device *hdev, enum bpf_hid_attach_type type);
+};
+
+#ifdef CONFIG_BPF
+int bpf_hid_init(struct hid_device *hdev);
+void bpf_hid_exit(struct hid_device *hdev);
+void bpf_hid_set_hooks(struct bpf_hid_hooks *hooks);
+#else
+static inline int bpf_hid_init(struct hid_device *hdev)
+{
+ return 0;
+}
+
+static inline void bpf_hid_exit(struct hid_device *hdev) {}
+static inline void bpf_hid_set_hooks(struct bpf_hid_hooks *hooks) {}
+#endif
+
+#endif /* _BPF_HID_H */
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 48a91c51c015..1509862aacc4 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -76,6 +76,10 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_EXT, bpf_extension,
BPF_PROG_TYPE(BPF_PROG_TYPE_LSM, lsm,
void *, void *)
#endif /* CONFIG_BPF_LSM */
+#if IS_ENABLED(CONFIG_HID)
+BPF_PROG_TYPE(BPF_PROG_TYPE_HID, hid,
+ __u32, u32)
+#endif
#endif
BPF_PROG_TYPE(BPF_PROG_TYPE_SYSCALL, bpf_syscall,
void *, void *)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 7487b0586fe6..8fd79011f461 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -15,6 +15,7 @@


#include <linux/bitops.h>
+#include <linux/bpf-hid.h>
#include <linux/types.h>
#include <linux/slab.h>
#include <linux/list.h>
@@ -26,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
@@ -639,6 +641,10 @@ struct hid_device { /* device report descriptor */
struct list_head debug_list;
spinlock_t debug_list_lock;
wait_queue_head_t debug_wait;
+
+#ifdef CONFIG_BPF
+ struct bpf_hid bpf;
+#endif
};

#define to_hid_device(pdev) \
@@ -1205,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
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index afe3d0d7f5f2..5978b92cacd3 100644
--- a/include/uapi/linux/bpf.h
+++ b/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,7 @@ enum bpf_attach_type {
BPF_SK_REUSEPORT_SELECT,
BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
BPF_PERF_EVENT,
+ BPF_HID_DEVICE_EVENT,
__MAX_BPF_ATTACH_TYPE
};

@@ -1011,6 +1013,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,
};
@@ -5870,6 +5873,10 @@ struct bpf_link_info {
struct {
__u32 ifindex;
} xdp;
+ struct {
+ __s32 hidraw_ino;
+ __u32 attach_type;
+ } hid;
};
} __attribute__((aligned(8)));

diff --git a/include/uapi/linux/bpf_hid.h b/include/uapi/linux/bpf_hid.h
new file mode 100644
index 000000000000..243ac45a253f
--- /dev/null
+++ b/include/uapi/linux/bpf_hid.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */
+
+/*
+ * HID BPF public headers
+ *
+ * Copyright (c) 2021 Benjamin Tissoires
+ */
+
+#ifndef _UAPI__LINUX_BPF_HID_H__
+#define _UAPI__LINUX_BPF_HID_H__
+
+#include <linux/types.h>
+
+#define HID_BPF_MAX_BUFFER_SIZE 16384 /* 16kb */
+
+struct hid_device;
+
+enum hid_bpf_event {
+ HID_BPF_UNDEF = 0,
+ HID_BPF_DEVICE_EVENT,
+};
+
+/* type is HID_BPF_DEVICE_EVENT */
+struct hid_bpf_ctx_device_event {
+ __u8 data[HID_BPF_MAX_BUFFER_SIZE];
+ unsigned long size;
+};
+
+struct hid_bpf_ctx {
+ enum hid_bpf_event type;
+ struct hid_device *hdev;
+
+ union {
+ struct hid_bpf_ctx_device_event device;
+ } u;
+};
+
+#endif /* _UAPI__LINUX_BPF_HID_H__ */
+
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index c1a9be6a4b9f..8d5619d3d7e5 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -35,6 +35,9 @@ ifeq ($(CONFIG_BPF_JIT),y)
obj-$(CONFIG_BPF_SYSCALL) += bpf_struct_ops.o
obj-${CONFIG_BPF_LSM} += bpf_lsm.o
endif
+ifneq ($(CONFIG_HID),)
+obj-$(CONFIG_BPF_SYSCALL) += hid.o
+endif
obj-$(CONFIG_BPF_PRELOAD) += preload/

obj-$(CONFIG_BPF_SYSCALL) += relo_core.o
diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c
new file mode 100644
index 000000000000..d3cb952bfc26
--- /dev/null
+++ b/kernel/bpf/hid.c
@@ -0,0 +1,437 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * based on kernel/bpf/net-namespace.c
+ */
+
+#include <linux/bpf.h>
+#include <linux/bpf-hid.h>
+#include <linux/filter.h>
+#include <linux/hid.h>
+#include <linux/hidraw.h>
+
+/*
+ * Functions to manage BPF programs attached to hid
+ */
+
+struct bpf_hid_link {
+ struct bpf_link link;
+ enum bpf_attach_type type;
+ enum bpf_hid_attach_type hid_type;
+
+ /* Must be accessed with bpf_hid_mutex held. */
+ struct hid_device *hdev;
+ struct list_head node; /* node in list of links attached to hid */
+};
+
+/* Protects updates to bpf_hid */
+DEFINE_MUTEX(bpf_hid_mutex);
+
+static struct bpf_hid_hooks hid_hooks = {0};
+
+void bpf_hid_set_hooks(struct bpf_hid_hooks *hooks)
+{
+ if (hooks)
+ hid_hooks = *hooks;
+ else
+ memset(&hid_hooks, 0, sizeof(hid_hooks));
+}
+EXPORT_SYMBOL_GPL(bpf_hid_set_hooks);
+
+static const struct bpf_func_proto *
+hid_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ switch (func_id) {
+ default:
+ return bpf_base_func_proto(func_id);
+ }
+}
+
+static bool hid_is_valid_access(int off, int size,
+ enum bpf_access_type access_type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ /* everything not in ctx is prohibited */
+ if (off < 0 || off + size > sizeof(struct hid_bpf_ctx))
+ return false;
+
+ switch (off) {
+ /* type, hdev are read-only */
+ case bpf_ctx_range_till(struct hid_bpf_ctx, type, hdev):
+ return access_type == BPF_READ;
+ }
+
+ /* everything else is read/write */
+ return true;
+}
+
+const struct bpf_verifier_ops hid_verifier_ops = {
+ .get_func_proto = hid_func_proto,
+ .is_valid_access = hid_is_valid_access
+};
+
+/* Must be called with bpf_hid_mutex held. */
+static void bpf_hid_run_array_detach(struct hid_device *hdev,
+ enum bpf_hid_attach_type type)
+{
+ struct bpf_prog_array *run_array;
+
+ run_array = rcu_replace_pointer(hdev->bpf.run_array[type], NULL,
+ lockdep_is_held(&bpf_hid_mutex));
+ bpf_prog_array_free(run_array);
+
+ if (hid_hooks.array_detached)
+ hid_hooks.array_detached(hdev, type);
+}
+
+static int link_index(struct hid_device *hdev, enum bpf_hid_attach_type type,
+ struct bpf_hid_link *link)
+{
+ struct bpf_hid_link *pos;
+ int i = 0;
+
+ list_for_each_entry(pos, &hdev->bpf.links[type], node) {
+ if (pos == link)
+ return i;
+ i++;
+ }
+ return -ENOENT;
+}
+
+static int link_count(struct hid_device *hdev, enum bpf_hid_attach_type type)
+{
+ struct list_head *pos;
+ int i = 0;
+
+ list_for_each(pos, &hdev->bpf.links[type])
+ i++;
+ return i;
+}
+
+static void fill_prog_array(struct hid_device *hdev, enum bpf_hid_attach_type type,
+ struct bpf_prog_array *prog_array)
+{
+ struct bpf_hid_link *pos;
+ unsigned int i = 0;
+
+ list_for_each_entry(pos, &hdev->bpf.links[type], node) {
+ prog_array->items[i].prog = pos->link.prog;
+ i++;
+ }
+}
+
+static void bpf_hid_link_release(struct bpf_link *link)
+{
+ struct bpf_hid_link *hid_link =
+ container_of(link, struct bpf_hid_link, link);
+ enum bpf_hid_attach_type type = hid_link->hid_type;
+ struct bpf_prog_array *old_array, *new_array;
+ struct hid_device *hdev;
+ int cnt, idx;
+
+ mutex_lock(&bpf_hid_mutex);
+
+ hdev = hid_link->hdev;
+ if (!hdev)
+ goto out_unlock;
+
+ /* Remember link position in case of safe delete */
+ idx = link_index(hdev, type, hid_link);
+ list_del(&hid_link->node);
+
+ cnt = link_count(hdev, type);
+ if (!cnt) {
+ bpf_hid_run_array_detach(hdev, type);
+ goto out_unlock;
+ }
+
+ old_array = rcu_dereference_protected(hdev->bpf.run_array[type],
+ lockdep_is_held(&bpf_hid_mutex));
+ new_array = bpf_prog_array_alloc(cnt, GFP_KERNEL);
+ if (!new_array) {
+ WARN_ON(bpf_prog_array_delete_safe_at(old_array, idx));
+ goto out_unlock;
+ }
+ fill_prog_array(hdev, type, new_array);
+ rcu_assign_pointer(hdev->bpf.run_array[type], new_array);
+ bpf_prog_array_free(old_array);
+
+out_unlock:
+ hid_link->hdev = NULL;
+ mutex_unlock(&bpf_hid_mutex);
+}
+
+static int bpf_hid_link_detach(struct bpf_link *link)
+{
+ bpf_hid_link_release(link);
+ return 0;
+}
+
+static void bpf_hid_link_dealloc(struct bpf_link *link)
+{
+ struct bpf_hid_link *hid_link =
+ container_of(link, struct bpf_hid_link, link);
+
+ kfree(hid_link);
+}
+
+static int bpf_hid_link_update_prog(struct bpf_link *link,
+ struct bpf_prog *new_prog,
+ struct bpf_prog *old_prog)
+{
+ struct bpf_hid_link *hid_link =
+ container_of(link, struct bpf_hid_link, link);
+ enum bpf_hid_attach_type type = hid_link->hid_type;
+ struct bpf_prog_array *run_array;
+ struct hid_device *hdev;
+ int idx, ret;
+
+ if (old_prog && old_prog != link->prog)
+ return -EPERM;
+ if (new_prog->type != link->prog->type)
+ return -EINVAL;
+
+ mutex_lock(&bpf_hid_mutex);
+
+ hdev = hid_link->hdev;
+ if (!hdev) {
+ /* hid dying */
+ ret = -ENOLINK;
+ goto out_unlock;
+ }
+
+ run_array = rcu_dereference_protected(hdev->bpf.run_array[type],
+ lockdep_is_held(&bpf_hid_mutex));
+ idx = link_index(hdev, type, hid_link);
+ ret = bpf_prog_array_update_at(run_array, idx, new_prog);
+ if (ret)
+ goto out_unlock;
+
+ old_prog = xchg(&link->prog, new_prog);
+ bpf_prog_put(old_prog);
+
+out_unlock:
+ mutex_unlock(&bpf_hid_mutex);
+ return ret;
+}
+
+static int bpf_hid_link_fill_info(const struct bpf_link *link,
+ struct bpf_link_info *info)
+{
+ const struct bpf_hid_link *hid_link =
+ container_of(link, struct bpf_hid_link, link);
+ int hidraw_ino = -1;
+ struct hid_device *hdev;
+ struct hidraw *hidraw;
+
+ mutex_lock(&bpf_hid_mutex);
+ hdev = hid_link->hdev;
+ if (hdev && hdev->hidraw) {
+ hidraw = hdev->hidraw;
+ hidraw_ino = hidraw->minor;
+ }
+ mutex_unlock(&bpf_hid_mutex);
+
+ info->hid.hidraw_ino = hidraw_ino;
+ info->hid.attach_type = hid_link->type;
+ return 0;
+}
+
+static void bpf_hid_link_show_fdinfo(const struct bpf_link *link,
+ struct seq_file *seq)
+{
+ struct bpf_link_info info = {};
+
+ bpf_hid_link_fill_info(link, &info);
+ seq_printf(seq,
+ "hidraw_ino:\t%u\n"
+ "attach_type:\t%u\n",
+ info.hid.hidraw_ino,
+ info.hid.attach_type);
+}
+
+static const struct bpf_link_ops bpf_hid_link_ops = {
+ .release = bpf_hid_link_release,
+ .dealloc = bpf_hid_link_dealloc,
+ .detach = bpf_hid_link_detach,
+ .update_prog = bpf_hid_link_update_prog,
+ .fill_link_info = bpf_hid_link_fill_info,
+ .show_fdinfo = bpf_hid_link_show_fdinfo,
+};
+
+/* Must be called with bpf_hid_mutex held. */
+static int __bpf_hid_prog_query(const union bpf_attr *attr,
+ union bpf_attr __user *uattr,
+ struct hid_device *hdev,
+ enum bpf_hid_attach_type type)
+{
+ __u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
+ struct bpf_prog_array *run_array;
+ u32 prog_cnt = 0, flags = 0;
+
+ run_array = rcu_dereference_protected(hdev->bpf.run_array[type],
+ lockdep_is_held(&bpf_hid_mutex));
+ if (run_array)
+ prog_cnt = bpf_prog_array_length(run_array);
+
+ if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags)))
+ return -EFAULT;
+ if (copy_to_user(&uattr->query.prog_cnt, &prog_cnt, sizeof(prog_cnt)))
+ return -EFAULT;
+ if (!attr->query.prog_cnt || !prog_ids || !prog_cnt)
+ return 0;
+
+ return bpf_prog_array_copy_to_user(run_array, prog_ids,
+ attr->query.prog_cnt);
+}
+
+int bpf_hid_prog_query(const union bpf_attr *attr,
+ union bpf_attr __user *uattr)
+{
+ enum bpf_hid_attach_type type;
+ struct hid_device *hdev;
+ int ret;
+
+ if (attr->query.query_flags || !hid_hooks.hdev_from_fd)
+ return -EINVAL;
+
+ type = to_bpf_hid_attach_type(attr->query.attach_type);
+ if (type < 0)
+ return -EINVAL;
+
+ hdev = hid_hooks.hdev_from_fd(attr->query.target_fd);
+ if (IS_ERR(hdev))
+ return PTR_ERR(hdev);
+
+ mutex_lock(&bpf_hid_mutex);
+ ret = __bpf_hid_prog_query(attr, uattr, hdev, type);
+ mutex_unlock(&bpf_hid_mutex);
+
+ return ret;
+}
+
+static int bpf_hid_max_progs(enum bpf_hid_attach_type type)
+{
+ switch (type) {
+ case BPF_HID_ATTACH_DEVICE_EVENT:
+ return 64;
+ default:
+ return 0;
+ }
+}
+
+static int bpf_hid_link_attach(struct hid_device *hdev, struct bpf_link *link,
+ enum bpf_hid_attach_type type)
+{
+ struct bpf_hid_link *hid_link =
+ container_of(link, struct bpf_hid_link, link);
+ struct bpf_prog_array *run_array;
+ int cnt, err = 0;
+
+ mutex_lock(&bpf_hid_mutex);
+
+ cnt = link_count(hdev, type);
+ if (cnt >= bpf_hid_max_progs(type)) {
+ err = -E2BIG;
+ goto out_unlock;
+ }
+
+ if (hid_hooks.link_attach) {
+ err = hid_hooks.link_attach(hdev, type);
+ if (err)
+ goto out_unlock;
+ }
+
+ run_array = bpf_prog_array_alloc(cnt + 1, GFP_KERNEL);
+ if (!run_array) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ list_add_tail(&hid_link->node, &hdev->bpf.links[type]);
+
+ fill_prog_array(hdev, type, run_array);
+ run_array = rcu_replace_pointer(hdev->bpf.run_array[type], run_array,
+ lockdep_is_held(&bpf_hid_mutex));
+ bpf_prog_array_free(run_array);
+
+out_unlock:
+ mutex_unlock(&bpf_hid_mutex);
+ return err;
+}
+
+int bpf_hid_link_create(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+ enum bpf_hid_attach_type hid_type;
+ struct bpf_link_primer link_primer;
+ struct bpf_hid_link *hid_link;
+ enum bpf_attach_type type;
+ struct hid_device *hdev;
+ int err;
+
+ if (attr->link_create.flags || !hid_hooks.hdev_from_fd)
+ return -EINVAL;
+
+ type = attr->link_create.attach_type;
+ hid_type = to_bpf_hid_attach_type(type);
+ if (hid_type < 0)
+ return -EINVAL;
+
+ hdev = hid_hooks.hdev_from_fd(attr->link_create.target_fd);
+ if (IS_ERR(hdev))
+ return PTR_ERR(hdev);
+
+ hid_link = kzalloc(sizeof(*hid_link), GFP_USER);
+ if (!hid_link)
+ return -ENOMEM;
+
+ bpf_link_init(&hid_link->link, BPF_LINK_TYPE_HID,
+ &bpf_hid_link_ops, prog);
+ hid_link->hdev = hdev;
+ hid_link->type = type;
+ hid_link->hid_type = hid_type;
+
+ err = bpf_link_prime(&hid_link->link, &link_primer);
+ if (err) {
+ kfree(hid_link);
+ return err;
+ }
+
+ err = bpf_hid_link_attach(hdev, &hid_link->link, hid_type);
+ if (err) {
+ bpf_link_cleanup(&link_primer);
+ return err;
+ }
+
+ return bpf_link_settle(&link_primer);
+}
+
+const struct bpf_prog_ops hid_prog_ops = {
+};
+
+int bpf_hid_init(struct hid_device *hdev)
+{
+ int type;
+
+ for (type = 0; type < MAX_BPF_HID_ATTACH_TYPE; type++)
+ INIT_LIST_HEAD(&hdev->bpf.links[type]);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(bpf_hid_init);
+
+void bpf_hid_exit(struct hid_device *hdev)
+{
+ enum bpf_hid_attach_type type;
+ struct bpf_hid_link *hid_link;
+
+ mutex_lock(&bpf_hid_mutex);
+ for (type = 0; type < MAX_BPF_HID_ATTACH_TYPE; type++) {
+ bpf_hid_run_array_detach(hdev, type);
+ list_for_each_entry(hid_link, &hdev->bpf.links[type], node) {
+ hid_link->hdev = NULL; /* auto-detach link */
+ }
+ }
+ mutex_unlock(&bpf_hid_mutex);
+}
+EXPORT_SYMBOL_GPL(bpf_hid_exit);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 9c7a72b65eee..230ca6964a7e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3,6 +3,7 @@
*/
#include <linux/bpf.h>
#include <linux/bpf-cgroup.h>
+#include <linux/bpf-hid.h>
#include <linux/bpf_trace.h>
#include <linux/bpf_lirc.h>
#include <linux/bpf_verifier.h>
@@ -2174,6 +2175,7 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type)
case BPF_PROG_TYPE_CGROUP_SYSCTL:
case BPF_PROG_TYPE_SOCK_OPS:
case BPF_PROG_TYPE_EXT: /* extends any prog */
+ case BPF_PROG_TYPE_HID:
return true;
case BPF_PROG_TYPE_CGROUP_SKB:
/* always unpriv */
@@ -3188,6 +3190,8 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
return BPF_PROG_TYPE_SK_LOOKUP;
case BPF_XDP:
return BPF_PROG_TYPE_XDP;
+ case BPF_HID_DEVICE_EVENT:
+ return BPF_PROG_TYPE_HID;
default:
return BPF_PROG_TYPE_UNSPEC;
}
@@ -3331,6 +3335,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
case BPF_SK_MSG_VERDICT:
case BPF_SK_SKB_VERDICT:
return sock_map_bpf_prog_query(attr, uattr);
+ case BPF_HID_DEVICE_EVENT:
+ return bpf_hid_prog_query(attr, uattr);
default:
return -EINVAL;
}
@@ -4325,6 +4331,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
ret = bpf_perf_link_attach(attr, prog);
break;
#endif
+ case BPF_PROG_TYPE_HID:
+ return bpf_hid_link_create(attr, prog);
default:
ret = -EINVAL;
}
diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
index 0e7bfdbff80a..65440bd618b2 100644
--- a/samples/bpf/.gitignore
+++ b/samples/bpf/.gitignore
@@ -2,6 +2,7 @@
cpustat
fds_example
hbm
+hid_mouse
ibumad
lathist
lwt_len_hist
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 38638845db9d..84ef458487df 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -60,6 +60,8 @@ tprogs-y += xdp_redirect_map
tprogs-y += xdp_redirect
tprogs-y += xdp_monitor

+tprogs-y += hid_mouse
+
# Libbpf dependencies
LIBBPF_SRC = $(TOOLS_PATH)/lib/bpf
LIBBPF_OUTPUT = $(abspath $(BPF_SAMPLES_PATH))/libbpf
@@ -124,6 +126,7 @@ xdp_redirect_cpu-objs := xdp_redirect_cpu_user.o $(XDP_SAMPLE)
xdp_redirect_map-objs := xdp_redirect_map_user.o $(XDP_SAMPLE)
xdp_redirect-objs := xdp_redirect_user.o $(XDP_SAMPLE)
xdp_monitor-objs := xdp_monitor_user.o $(XDP_SAMPLE)
+hid_mouse-objs := hid_mouse_user.o

# Tell kbuild to always build the programs
always-y := $(tprogs-y)
@@ -181,6 +184,7 @@ always-y += ibumad_kern.o
always-y += hbm_out_kern.o
always-y += hbm_edt_kern.o
always-y += xdpsock_kern.o
+always-y += hid_mouse_kern.o

ifeq ($(ARCH), arm)
# Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
diff --git a/samples/bpf/hid_mouse_kern.c b/samples/bpf/hid_mouse_kern.c
new file mode 100644
index 000000000000..83b0ab5a04d0
--- /dev/null
+++ b/samples/bpf/hid_mouse_kern.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2021 Benjamin Tissoires
+ */
+#include <linux/version.h>
+#include <uapi/linux/bpf.h>
+#include <uapi/linux/bpf_hid.h>
+#include <bpf/bpf_helpers.h>
+
+SEC("hid/device_event")
+int hid_y_event(struct hid_bpf_ctx *ctx)
+{
+ s16 y;
+
+ bpf_printk("event: %02x size: %d", ctx->type, ctx->u.device.size);
+ bpf_printk("incoming event: %02x %02x %02x",
+ ctx->u.device.data[0],
+ ctx->u.device.data[1],
+ ctx->u.device.data[2]);
+ bpf_printk(" %02x %02x %02x",
+ ctx->u.device.data[3],
+ ctx->u.device.data[4],
+ ctx->u.device.data[5]);
+ bpf_printk(" %02x %02x %02x",
+ ctx->u.device.data[6],
+ ctx->u.device.data[7],
+ ctx->u.device.data[8]);
+
+ y = ctx->u.device.data[3] | (ctx->u.device.data[4] << 8);
+
+ y = -y;
+
+ ctx->u.device.data[3] = y & 0xFF;
+ ctx->u.device.data[4] = (y >> 8) & 0xFF;
+
+ bpf_printk("modified event: %02x %02x %02x",
+ ctx->u.device.data[0],
+ ctx->u.device.data[1],
+ ctx->u.device.data[2]);
+ bpf_printk(" %02x %02x %02x",
+ ctx->u.device.data[3],
+ ctx->u.device.data[4],
+ ctx->u.device.data[5]);
+ bpf_printk(" %02x %02x %02x",
+ ctx->u.device.data[6],
+ ctx->u.device.data[7],
+ ctx->u.device.data[8]);
+
+ return 0;
+}
+
+SEC("hid/device_event")
+int hid_x_event(struct hid_bpf_ctx *ctx)
+{
+ s16 x;
+
+ x = ctx->u.device.data[1] | (ctx->u.device.data[2] << 8);
+
+ x = -x;
+
+ ctx->u.device.data[1] = x & 0xFF;
+ ctx->u.device.data[2] = (x >> 8) & 0xFF;
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/samples/bpf/hid_mouse_user.c b/samples/bpf/hid_mouse_user.c
new file mode 100644
index 000000000000..d4f37caca2fa
--- /dev/null
+++ b/samples/bpf/hid_mouse_user.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2021 Benjamin Tissoires
+ */
+#include <linux/bpf.h>
+#include <linux/if_link.h>
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <libgen.h>
+#include <sys/resource.h>
+
+#include "bpf_util.h"
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+static char *sysfs_path;
+static int sysfs_fd;
+static int prog_count;
+
+struct prog {
+ int fd;
+ struct bpf_link *link;
+ enum bpf_attach_type type;
+};
+
+static struct prog progs[10];
+
+static void int_exit(int sig)
+{
+ for (prog_count--; prog_count >= 0; prog_count--)
+ bpf_link__destroy(progs[prog_count].link);
+
+ close(sysfs_fd);
+ exit(0);
+}
+
+static void usage(const char *prog)
+{
+ fprintf(stderr,
+ "%s: %s /sys/bus/hid/devices/0BUS:0VID:0PID:00ID/uevent\n\n",
+ __func__, prog);
+}
+
+int main(int argc, char **argv)
+{
+ struct bpf_prog_info info = {};
+ __u32 info_len = sizeof(info);
+ const char *optstr = "";
+ struct bpf_object *obj;
+ struct bpf_program *prog;
+ int opt;
+ char filename[256];
+ int err;
+
+ while ((opt = getopt(argc, argv, optstr)) != -1) {
+ switch (opt) {
+ default:
+ usage(basename(argv[0]));
+ return 1;
+ }
+ }
+
+ if (optind == argc) {
+ usage(basename(argv[0]));
+ return 1;
+ }
+
+ sysfs_path = argv[optind];
+ if (!sysfs_path) {
+ perror("sysfs");
+ return 1;
+ }
+
+ snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+ obj = bpf_object__open_file(filename, NULL);
+ err = libbpf_get_error(obj);
+ if (err) {
+ fprintf(stderr, "ERROR: opening BPF object file failed\n");
+ obj = NULL;
+ err = 1;
+ goto cleanup;
+ }
+
+ /* load BPF program */
+ err = bpf_object__load(obj);
+ if (err) {
+ fprintf(stderr, "ERROR: loading BPF object file failed\n");
+ goto cleanup;
+ }
+
+ sysfs_fd = open(sysfs_path, O_RDONLY);
+
+ bpf_object__for_each_program(prog, obj) {
+ progs[prog_count].fd = bpf_program__fd(prog);
+ progs[prog_count].type = bpf_program__get_expected_attach_type(prog);
+ progs[prog_count].link = bpf_program__attach_hid(prog, sysfs_fd);
+ if (libbpf_get_error(progs[prog_count].link)) {
+ fprintf(stderr, "bpf_prog_attach: err=%m\n");
+ progs[prog_count].fd = 0;
+ progs[prog_count].link = NULL;
+ goto cleanup;
+ }
+ prog_count++;
+ }
+
+ signal(SIGINT, int_exit);
+ signal(SIGTERM, int_exit);
+
+ err = bpf_obj_get_info_by_fd(progs[0].fd, &info, &info_len);
+ if (err) {
+ printf("can't get prog info - %s\n", strerror(errno));
+ goto cleanup;
+ }
+
+ while (1)
+ ;
+
+ cleanup:
+ for (prog_count--; prog_count >= 0; prog_count--)
+ bpf_link__destroy(progs[prog_count].link);
+
+ bpf_object__close(obj);
+ return err;
+}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index afe3d0d7f5f2..5978b92cacd3 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,7 @@ enum bpf_attach_type {
BPF_SK_REUSEPORT_SELECT,
BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
BPF_PERF_EVENT,
+ BPF_HID_DEVICE_EVENT,
__MAX_BPF_ATTACH_TYPE
};

@@ -1011,6 +1013,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,
};
@@ -5870,6 +5873,10 @@ struct bpf_link_info {
struct {
__u32 ifindex;
} xdp;
+ struct {
+ __s32 hidraw_ino;
+ __u32 attach_type;
+ } hid;
};
} __attribute__((aligned(8)));

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7e978feaf822..bad16e85032e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8676,6 +8676,7 @@ 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_SLOPPY_PFX),
};

#define MAX_TYPE_NAME_SIZE 32
@@ -10655,6 +10656,12 @@ static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie)
return bpf_program__attach_iter(prog, NULL);
}

+struct bpf_link *
+bpf_program__attach_hid(const struct bpf_program *prog, int hid_fd)
+{
+ return bpf_program__attach_fd(prog, hid_fd, 0, "hid");
+}
+
struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
{
if (!prog->sec_def || !prog->sec_def->attach_fn)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index c8d8daad212e..f677ac0a9ede 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);

/*
* 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 47e70c9058d9..fdc6fa743953 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -424,6 +424,7 @@ LIBBPF_0.6.0 {
LIBBPF_0.7.0 {
global:
bpf_btf_load;
+ bpf_program__attach_hid;
bpf_program__expected_attach_type;
bpf_program__log_buf;
bpf_program__log_level;
diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
new file mode 100644
index 000000000000..692d78b9dc4a
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/hid.c
@@ -0,0 +1,318 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Red Hat */
+#include <test_progs.h>
+#include <testing_helpers.h>
+#include "hid.skel.h"
+
+#include <fcntl.h>
+#include <fnmatch.h>
+#include <dirent.h>
+#include <poll.h>
+#include <stdbool.h>
+#include <linux/uhid.h>
+
+static unsigned char rdesc[] = {
+ 0x06, 0x00, 0xff, /* Usage Page (Vendor Defined Page 1) */
+ 0x09, 0x21, /* Usage (Vendor Usage 0x21) */
+ 0xa1, 0x01, /* COLLECTION (Application) */
+ 0x09, 0x01, /* Usage (Vendor Usage 0x01) */
+ 0xa1, 0x00, /* COLLECTION (Physical) */
+ 0x85, 0x01, /* REPORT_ID (1) */
+ 0x06, 0x00, 0xff, /* Usage Page (Vendor Defined Page 1) */
+ 0x19, 0x01, /* USAGE_MINIMUM (1) */
+ 0x29, 0x03, /* USAGE_MAXIMUM (3) */
+ 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
+ 0x25, 0x01, /* LOGICAL_MAXIMUM (1) */
+ 0x95, 0x03, /* REPORT_COUNT (3) */
+ 0x75, 0x01, /* REPORT_SIZE (1) */
+ 0x81, 0x02, /* INPUT (Data,Var,Abs) */
+ 0x95, 0x01, /* REPORT_COUNT (1) */
+ 0x75, 0x05, /* REPORT_SIZE (5) */
+ 0x81, 0x01, /* INPUT (Cnst,Var,Abs) */
+ 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */
+ 0x09, 0x30, /* USAGE (X) */
+ 0x09, 0x31, /* USAGE (Y) */
+ 0x15, 0x81, /* LOGICAL_MINIMUM (-127) */
+ 0x25, 0x7f, /* LOGICAL_MAXIMUM (127) */
+ 0x75, 0x08, /* REPORT_SIZE (8) */
+ 0x95, 0x02, /* REPORT_COUNT (2) */
+ 0x81, 0x06, /* INPUT (Data,Var,Rel) */
+
+ 0x06, 0x00, 0xff, /* Usage Page (Vendor Defined Page 1) */
+ 0x19, 0x01, /* USAGE_MINIMUM (1) */
+ 0x29, 0x03, /* USAGE_MAXIMUM (3) */
+ 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
+ 0x25, 0x01, /* LOGICAL_MAXIMUM (1) */
+ 0x95, 0x03, /* REPORT_COUNT (3) */
+ 0x75, 0x01, /* REPORT_SIZE (1) */
+ 0x91, 0x02, /* Output (Data,Var,Abs) */
+ 0x95, 0x01, /* REPORT_COUNT (1) */
+ 0x75, 0x05, /* REPORT_SIZE (5) */
+ 0x91, 0x01, /* Output (Cnst,Var,Abs) */
+
+ 0x06, 0x00, 0xff, /* Usage Page (Vendor Defined Page 1) */
+ 0x19, 0x06, /* USAGE_MINIMUM (6) */
+ 0x29, 0x08, /* USAGE_MAXIMUM (8) */
+ 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
+ 0x25, 0x01, /* LOGICAL_MAXIMUM (1) */
+ 0x95, 0x03, /* REPORT_COUNT (3) */
+ 0x75, 0x01, /* REPORT_SIZE (1) */
+ 0xb1, 0x02, /* Feature (Data,Var,Abs) */
+ 0x95, 0x01, /* REPORT_COUNT (1) */
+ 0x75, 0x05, /* REPORT_SIZE (5) */
+ 0x91, 0x01, /* Output (Cnst,Var,Abs) */
+
+ 0xc0, /* END_COLLECTION */
+ 0xc0, /* END_COLLECTION */
+};
+
+static int uhid_write(int fd, const struct uhid_event *ev)
+{
+ ssize_t ret;
+
+ ret = write(fd, ev, sizeof(*ev));
+ if (ret < 0) {
+ fprintf(stderr, "Cannot write to uhid: %m\n");
+ return -errno;
+ } else if (ret != sizeof(*ev)) {
+ fprintf(stderr, "Wrong size written to uhid: %zd != %zu\n",
+ ret, sizeof(ev));
+ return -EFAULT;
+ } else {
+ return 0;
+ }
+}
+
+static int create(int fd, int rand_nb)
+{
+ struct uhid_event ev;
+ char buf[25];
+
+ sprintf(buf, "test-uhid-device-%d", rand_nb);
+
+ memset(&ev, 0, sizeof(ev));
+ ev.type = UHID_CREATE;
+ strcpy((char *)ev.u.create.name, buf);
+ ev.u.create.rd_data = rdesc;
+ ev.u.create.rd_size = sizeof(rdesc);
+ ev.u.create.bus = BUS_USB;
+ ev.u.create.vendor = 0x0001;
+ ev.u.create.product = 0x0a37;
+ ev.u.create.version = 0;
+ ev.u.create.country = 0;
+
+ sprintf(buf, "%d", rand_nb);
+ strcpy((char *)ev.u.create.phys, buf);
+
+ return uhid_write(fd, &ev);
+}
+
+static void destroy(int fd)
+{
+ struct uhid_event ev;
+
+ memset(&ev, 0, sizeof(ev));
+ ev.type = UHID_DESTROY;
+
+ uhid_write(fd, &ev);
+}
+
+static int send_event(int fd, u8 *buf, size_t size)
+{
+ struct uhid_event ev;
+
+ if (size > sizeof(ev.u.input.data))
+ return -E2BIG;
+
+ memset(&ev, 0, sizeof(ev));
+ ev.type = UHID_INPUT2;
+ ev.u.input2.size = size;
+
+ memcpy(ev.u.input2.data, buf, size);
+
+ return uhid_write(fd, &ev);
+}
+
+static int setup_uhid(int rand_nb)
+{
+ int fd;
+ const char *path = "/dev/uhid";
+ int ret;
+
+ fd = open(path, O_RDWR | O_CLOEXEC);
+ if (!ASSERT_GE(fd, 0, "open uhid-cdev"))
+ return -EPERM;
+
+ ret = create(fd, rand_nb);
+ if (!ASSERT_OK(ret, "create uhid device")) {
+ close(fd);
+ return -EPERM;
+ }
+
+ return fd;
+}
+
+static int get_sysfs_fd(int rand_nb)
+{
+ const char *workdir = "/sys/devices/virtual/misc/uhid";
+ const char *target = "0003:0001:0A37.*";
+ char uevent[1024];
+ char temp[512];
+ char phys[512];
+ DIR *d;
+ struct dirent *dir;
+ int fd, nread;
+ int found = -1;
+
+ /* it would be nice to be able to use nftw, but the no_alu32 target doesn't support it */
+
+ sprintf(phys, "PHYS=%d", rand_nb);
+
+ d = opendir(workdir);
+ if (d) {
+ while ((dir = readdir(d)) != NULL) {
+ if (fnmatch(target, dir->d_name, 0))
+ continue;
+
+ /* we found the correct VID/PID, now check for phys */
+ sprintf(uevent, "%s/%s/uevent", workdir, dir->d_name);
+ fd = open(uevent, O_RDONLY | O_NONBLOCK);
+ if (fd < 0)
+ continue;
+
+ nread = read(fd, temp, ARRAY_SIZE(temp));
+ if (nread > 0 && (strstr(temp, phys)) != NULL) {
+ found = fd;
+ break;
+ }
+
+ close(fd);
+ fd = 0;
+ }
+ closedir(d);
+ }
+
+ return found;
+}
+
+static int get_hidraw(struct bpf_link *link)
+{
+ struct bpf_link_info info = {0};
+ int prog_id, i;
+
+ /* retry 5 times in case the system is loaded */
+ for (i = 5; i > 0; i--) {
+ usleep(10);
+ prog_id = link_info_prog_id(link, &info);
+ if (!prog_id)
+ continue;
+ if (info.hid.hidraw_ino >= 0)
+ break;
+ }
+
+ if (!prog_id)
+ return -1;
+
+ return info.hid.hidraw_ino;
+}
+
+/*
+ * Attach hid_first_event to the given uhid device,
+ * retrieve and open the matching hidraw node,
+ * inject one event in the uhid device,
+ * check that the program sees it and can change the data
+ */
+static int test_hid_raw_event(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
+{
+ int err, hidraw_ino, hidraw_fd = -1;
+ char hidraw_path[64] = {0};
+ u8 buf[10] = {0};
+ int ret = -1;
+
+ /* check that the program is correctly loaded */
+ ASSERT_EQ(hid_skel->data->callback_check, 52, "callback_check1");
+ ASSERT_EQ(hid_skel->data->callback2_check, 52, "callback2_check1");
+
+ /* attach the first program */
+ hid_skel->links.hid_first_event =
+ bpf_program__attach_hid(hid_skel->progs.hid_first_event, sysfs_fd);
+ if (!ASSERT_OK_PTR(hid_skel->links.hid_first_event,
+ "attach_hid(hid_first_event)"))
+ return PTR_ERR(hid_skel->links.hid_first_event);
+
+ hidraw_ino = get_hidraw(hid_skel->links.hid_first_event);
+ if (!ASSERT_GE(hidraw_ino, 0, "get_hidraw"))
+ goto cleanup;
+
+ /* open hidraw node to check the other side of the pipe */
+ sprintf(hidraw_path, "/dev/hidraw%d", hidraw_ino);
+ hidraw_fd = open(hidraw_path, O_RDWR | O_NONBLOCK);
+
+ if (!ASSERT_GE(hidraw_fd, 0, "open_hidraw"))
+ goto cleanup;
+
+ /* inject one event */
+ buf[0] = 1;
+ buf[1] = 42;
+ send_event(uhid_fd, buf, 4);
+
+ /* check that hid_first_event() was executed */
+ ASSERT_EQ(hid_skel->data->callback_check, 42, "callback_check1");
+
+ /* read the data from hidraw */
+ memset(buf, 0, sizeof(buf));
+ err = read(hidraw_fd, buf, sizeof(buf));
+ if (!ASSERT_EQ(err, 4, "read_hidraw"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[2], 47, "hid_first_event"))
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ if (hidraw_fd >= 0)
+ close(hidraw_fd);
+
+ hid__detach(hid_skel);
+
+ return ret;
+}
+
+void serial_test_hid_bpf(void)
+{
+ struct hid *hid_skel = NULL;
+ int err, uhid_fd, sysfs_fd;
+ time_t t;
+ int rand_nb;
+
+ /* initialize random number generator */
+ srand((unsigned int)time(&t));
+
+ rand_nb = rand() % 1024;
+
+ uhid_fd = setup_uhid(rand_nb);
+ if (!ASSERT_GE(uhid_fd, 0, "setup uhid"))
+ return;
+
+ /* give a little bit of time for the device to appear */
+ /* TODO: check on uhid events */
+ usleep(1000);
+
+ /* locate the uevent file of the created device */
+ sysfs_fd = get_sysfs_fd(rand_nb);
+ if (!ASSERT_GE(sysfs_fd, 0, "locate sysfs uhid device"))
+ goto cleanup;
+
+ hid_skel = hid__open_and_load();
+ if (!ASSERT_OK_PTR(hid_skel, "hid_skel_load"))
+ goto cleanup;
+
+ /* start the tests! */
+ err = test_hid_raw_event(hid_skel, uhid_fd, sysfs_fd);
+ ASSERT_OK(err, "hid");
+
+cleanup:
+ hid__destroy(hid_skel);
+ destroy(uhid_fd);
+}
diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
new file mode 100644
index 000000000000..f28bb6007875
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Red hat */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <linux/bpf_hid.h>
+
+char _license[] SEC("license") = "GPL";
+
+__u64 callback_check = 52;
+__u64 callback2_check = 52;
+
+SEC("hid/device_event")
+int hid_first_event(struct hid_bpf_ctx *ctx)
+{
+ callback_check = ctx->u.device.data[1];
+
+ ctx->u.device.data[2] = ctx->u.device.data[1] + 5;
+
+ return 0;
+}
--
2.35.1

2022-02-24 12:38:30

by Benjamin Tissoires

Subject: [PATCH bpf-next v1 2/6] HID: bpf: allow to change the report descriptor from an eBPF program

The report descriptor is the dictionary of the HID protocol specific
to the given device.
Changing it is a common habit in the HID world, and making that feature
accessible from eBPF allows to fix devices without having to install a
new kernel.

Signed-off-by: Benjamin Tissoires <[email protected]>
---
drivers/hid/hid-bpf.c | 66 +++++++++++++++++++
drivers/hid/hid-core.c | 3 +-
include/linux/bpf-hid.h | 4 ++
include/linux/hid.h | 6 ++
include/uapi/linux/bpf.h | 1 +
include/uapi/linux/bpf_hid.h | 8 +++
kernel/bpf/hid.c | 5 ++
kernel/bpf/syscall.c | 2 +
samples/bpf/hid_mouse_kern.c | 25 +++++++
tools/include/uapi/linux/bpf.h | 1 +
tools/lib/bpf/libbpf.c | 1 +
tools/testing/selftests/bpf/prog_tests/hid.c | 69 ++++++++++++++++++++
tools/testing/selftests/bpf/progs/hid.c | 48 ++++++++++++++
13 files changed, 238 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-bpf.c b/drivers/hid/hid-bpf.c
index 6c8445820944..2d54c87cda1a 100644
--- a/drivers/hid/hid-bpf.c
+++ b/drivers/hid/hid-bpf.c
@@ -63,6 +63,14 @@ static struct hid_bpf_ctx *hid_bpf_allocate_ctx(struct hid_device *hdev)
return ctx;
}

+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_link_attach(struct hid_device *hdev, enum bpf_hid_attach_type type)
{
int err = 0;
@@ -84,6 +92,17 @@ static int hid_bpf_link_attach(struct hid_device *hdev, enum bpf_hid_attach_type
return err;
}

+static void hid_bpf_link_attached(struct hid_device *hdev, enum bpf_hid_attach_type type)
+{
+ switch (type) {
+ case BPF_HID_ATTACH_RDESC_FIXUP:
+ hid_reconnect(hdev);
+ break;
+ default:
+ /* do nothing */
+ }
+}
+
static void hid_bpf_array_detached(struct hid_device *hdev, enum bpf_hid_attach_type type)
{
switch (type) {
@@ -91,6 +110,9 @@ static void hid_bpf_array_detached(struct hid_device *hdev, enum bpf_hid_attach_
kfree(hdev->bpf.ctx);
hdev->bpf.ctx = NULL;
break;
+ case BPF_HID_ATTACH_RDESC_FIXUP:
+ hid_reconnect(hdev);
+ break;
default:
/* do nothing */
}
@@ -110,6 +132,11 @@ static int hid_bpf_run_progs(struct hid_device *hdev, enum bpf_hid_attach_type t
if (size > sizeof(ctx->u.device.data))
return -E2BIG;
break;
+ case BPF_HID_ATTACH_RDESC_FIXUP:
+ event = HID_BPF_RDESC_FIXUP;
+ if (size > sizeof(ctx->u.rdesc.data))
+ return -E2BIG;
+ break;
default:
return -EINVAL;
}
@@ -127,6 +154,10 @@ static int hid_bpf_run_progs(struct hid_device *hdev, enum bpf_hid_attach_type t
memcpy(ctx->u.device.data, data, size);
ctx->u.device.size = size;
break;
+ case HID_BPF_RDESC_FIXUP:
+ memcpy(ctx->u.rdesc.data, data, size);
+ ctx->u.device.size = size;
+ break;
default:
/* do nothing */
}
@@ -157,11 +188,46 @@ u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *data, int *size)
return hdev->bpf.ctx->u.device.data;
}

+u8 *hid_bpf_report_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size)
+{
+ struct hid_bpf_ctx *ctx = NULL;
+ int ret;
+
+ if (bpf_hid_link_empty(&hdev->bpf, BPF_HID_ATTACH_RDESC_FIXUP))
+ goto ignore_bpf;
+
+ ctx = hid_bpf_allocate_ctx(hdev);
+ if (IS_ERR(ctx))
+ goto ignore_bpf;
+
+ ret = hid_bpf_run_progs(hdev, BPF_HID_ATTACH_RDESC_FIXUP, ctx, rdesc, *size);
+ if (ret)
+ goto ignore_bpf;
+
+ *size = ctx->u.rdesc.size;
+
+ if (!*size) {
+ rdesc = NULL;
+ goto unlock;
+ }
+
+ rdesc = kmemdup(ctx->u.rdesc.data, *size, GFP_KERNEL);
+
+ unlock:
+ kfree(ctx);
+ return rdesc;
+
+ ignore_bpf:
+ kfree(ctx);
+ 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,
.link_attach = hid_bpf_link_attach,
+ .link_attached = hid_bpf_link_attached,
.array_detached = hid_bpf_array_detached,
};

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a80bffe6ce4a..0eb8189faaee 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/bpf-hid.h b/include/linux/bpf-hid.h
index 363fb6a4923f..377012a019da 100644
--- a/include/linux/bpf-hid.h
+++ b/include/linux/bpf-hid.h
@@ -14,6 +14,7 @@ struct hid_device;
enum bpf_hid_attach_type {
BPF_HID_ATTACH_INVALID = -1,
BPF_HID_ATTACH_DEVICE_EVENT = 0,
+ BPF_HID_ATTACH_RDESC_FIXUP,
MAX_BPF_HID_ATTACH_TYPE
};

@@ -31,6 +32,8 @@ to_bpf_hid_attach_type(enum bpf_attach_type attach_type)
switch (attach_type) {
case BPF_HID_DEVICE_EVENT:
return BPF_HID_ATTACH_DEVICE_EVENT;
+ case BPF_HID_RDESC_FIXUP:
+ return BPF_HID_ATTACH_RDESC_FIXUP;
default:
return BPF_HID_ATTACH_INVALID;
}
@@ -67,6 +70,7 @@ static inline bool bpf_hid_link_empty(struct bpf_hid *bpf,
struct bpf_hid_hooks {
struct hid_device *(*hdev_from_fd)(int fd);
int (*link_attach)(struct hid_device *hdev, enum bpf_hid_attach_type type);
+ void (*link_attached)(struct hid_device *hdev, enum bpf_hid_attach_type type);
void (*array_detached)(struct hid_device *hdev, enum bpf_hid_attach_type type);
};

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
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 5978b92cacd3..a7a8d9cfcf24 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -999,6 +999,7 @@ enum bpf_attach_type {
BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
BPF_PERF_EVENT,
BPF_HID_DEVICE_EVENT,
+ BPF_HID_RDESC_FIXUP,
__MAX_BPF_ATTACH_TYPE
};

diff --git a/include/uapi/linux/bpf_hid.h b/include/uapi/linux/bpf_hid.h
index 243ac45a253f..c0801d7174c3 100644
--- a/include/uapi/linux/bpf_hid.h
+++ b/include/uapi/linux/bpf_hid.h
@@ -18,6 +18,7 @@ struct hid_device;
enum hid_bpf_event {
HID_BPF_UNDEF = 0,
HID_BPF_DEVICE_EVENT,
+ HID_BPF_RDESC_FIXUP,
};

/* type is HID_BPF_DEVICE_EVENT */
@@ -26,12 +27,19 @@ struct hid_bpf_ctx_device_event {
unsigned long size;
};

+/* type is HID_BPF_RDESC_FIXUP */
+struct hid_bpf_ctx_rdesc_fixup {
+ __u8 data[HID_BPF_MAX_BUFFER_SIZE];
+ unsigned long size;
+};
+
struct hid_bpf_ctx {
enum hid_bpf_event type;
struct hid_device *hdev;

union {
struct hid_bpf_ctx_device_event device;
+ struct hid_bpf_ctx_rdesc_fixup rdesc;
} u;
};

diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c
index d3cb952bfc26..47cb0580b14a 100644
--- a/kernel/bpf/hid.c
+++ b/kernel/bpf/hid.c
@@ -315,6 +315,8 @@ static int bpf_hid_max_progs(enum bpf_hid_attach_type type)
switch (type) {
case BPF_HID_ATTACH_DEVICE_EVENT:
return 64;
+ case BPF_HID_ATTACH_RDESC_FIXUP:
+ return 1;
default:
return 0;
}
@@ -355,6 +357,9 @@ static int bpf_hid_link_attach(struct hid_device *hdev, struct bpf_link *link,
lockdep_is_held(&bpf_hid_mutex));
bpf_prog_array_free(run_array);

+ if (hid_hooks.link_attached)
+ hid_hooks.link_attached(hdev, type);
+
out_unlock:
mutex_unlock(&bpf_hid_mutex);
return err;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 230ca6964a7e..62889cc71a02 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3191,6 +3191,7 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
case BPF_XDP:
return BPF_PROG_TYPE_XDP;
case BPF_HID_DEVICE_EVENT:
+ case BPF_HID_RDESC_FIXUP:
return BPF_PROG_TYPE_HID;
default:
return BPF_PROG_TYPE_UNSPEC;
@@ -3336,6 +3337,7 @@ static int bpf_prog_query(const union bpf_attr *attr,
case BPF_SK_SKB_VERDICT:
return sock_map_bpf_prog_query(attr, uattr);
case BPF_HID_DEVICE_EVENT:
+ case BPF_HID_RDESC_FIXUP:
return bpf_hid_prog_query(attr, uattr);
default:
return -EINVAL;
diff --git a/samples/bpf/hid_mouse_kern.c b/samples/bpf/hid_mouse_kern.c
index 83b0ab5a04d0..b4fac1c8bf6e 100644
--- a/samples/bpf/hid_mouse_kern.c
+++ b/samples/bpf/hid_mouse_kern.c
@@ -62,5 +62,30 @@ int hid_x_event(struct hid_bpf_ctx *ctx)
return 0;
}

+SEC("hid/rdesc_fixup")
+int hid_rdesc_fixup(struct hid_bpf_ctx *ctx)
+{
+ if (ctx->type != HID_BPF_RDESC_FIXUP)
+ return 0;
+
+ bpf_printk("rdesc: %02x %02x %02x",
+ ctx->u.rdesc.data[0],
+ ctx->u.rdesc.data[1],
+ ctx->u.rdesc.data[2]);
+ bpf_printk(" %02x %02x %02x",
+ ctx->u.rdesc.data[3],
+ ctx->u.rdesc.data[4],
+ ctx->u.rdesc.data[5]);
+ bpf_printk(" %02x %02x %02x ...",
+ ctx->u.rdesc.data[6],
+ ctx->u.rdesc.data[7],
+ ctx->u.rdesc.data[8]);
+
+ ctx->u.rdesc.data[39] = 0x31;
+ ctx->u.rdesc.data[41] = 0x30;
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 5978b92cacd3..a7a8d9cfcf24 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -999,6 +999,7 @@ enum bpf_attach_type {
BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
BPF_PERF_EVENT,
BPF_HID_DEVICE_EVENT,
+ BPF_HID_RDESC_FIXUP,
__MAX_BPF_ATTACH_TYPE
};

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index bad16e85032e..b7af873116fb 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8677,6 +8677,7 @@ static const struct bpf_sec_def section_defs[] = {
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_SLOPPY_PFX),
+ SEC_DEF("hid/rdesc_fixup", HID, BPF_HID_RDESC_FIXUP, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX),
};

#define MAX_TYPE_NAME_SIZE 32
diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index 692d78b9dc4a..dccbbcaa69e5 100644
--- a/tools/testing/selftests/bpf/prog_tests/hid.c
+++ b/tools/testing/selftests/bpf/prog_tests/hid.c
@@ -9,6 +9,7 @@
#include <dirent.h>
#include <poll.h>
#include <stdbool.h>
+#include <linux/hidraw.h>
#include <linux/uhid.h>

static unsigned char rdesc[] = {
@@ -279,6 +280,71 @@ static int test_hid_raw_event(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
return ret;
}

+/*
+ * Attach hid_rdesc_fixup to the given uhid device,
+ * retrieve and open the matching hidraw node,
+ * check that the hidraw report descriptor has been updated.
+ */
+static int test_rdesc_fixup(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
+{
+ struct hidraw_report_descriptor rpt_desc = {0};
+ int err, desc_size, hidraw_ino, hidraw_fd = -1;
+ char hidraw_path[64] = {0};
+ int ret = -1;
+
+ /* attach the program */
+ hid_skel->links.hid_rdesc_fixup =
+ bpf_program__attach_hid(hid_skel->progs.hid_rdesc_fixup, sysfs_fd);
+ if (!ASSERT_OK_PTR(hid_skel->links.hid_rdesc_fixup,
+ "attach_hid(hid_rdesc_fixup)"))
+ return PTR_ERR(hid_skel->links.hid_rdesc_fixup);
+
+ /* give a little bit of time for the device to appear */
+ /* TODO: check on uhid events */
+ usleep(1000);
+
+ hidraw_ino = get_hidraw(hid_skel->links.hid_rdesc_fixup);
+ if (!ASSERT_GE(hidraw_ino, 0, "get_hidraw"))
+ goto cleanup;
+
+ /* open hidraw node to check the other side of the pipe */
+ sprintf(hidraw_path, "/dev/hidraw%d", hidraw_ino);
+ hidraw_fd = open(hidraw_path, O_RDWR | O_NONBLOCK);
+
+ if (!ASSERT_GE(hidraw_fd, 0, "open_hidraw"))
+ goto cleanup;
+
+ /* check that hid_rdesc_fixup() was executed */
+ ASSERT_EQ(hid_skel->data->callback2_check, 0x21, "callback_check2");
+
+ /* read the exposed report descriptor from hidraw */
+ err = ioctl(hidraw_fd, HIDIOCGRDESCSIZE, &desc_size);
+ if (!ASSERT_GE(err, 0, "HIDIOCGRDESCSIZE"))
+ goto cleanup;
+
+ /* ensure the new size of the rdesc is bigger than the old one */
+ if (!ASSERT_GT(desc_size, sizeof(rdesc), "new_rdesc_size"))
+ goto cleanup;
+
+ rpt_desc.size = desc_size;
+ err = ioctl(hidraw_fd, HIDIOCGRDESC, &rpt_desc);
+ if (!ASSERT_GE(err, 0, "HIDIOCGRDESC"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(rpt_desc.value[4], 0x42, "hid_rdesc_fixup"))
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ if (hidraw_fd >= 0)
+ close(hidraw_fd);
+
+ hid__detach(hid_skel);
+
+ return ret;
+}
+
void serial_test_hid_bpf(void)
{
struct hid *hid_skel = NULL;
@@ -312,6 +378,9 @@ void serial_test_hid_bpf(void)
err = test_hid_raw_event(hid_skel, uhid_fd, sysfs_fd);
ASSERT_OK(err, "hid");

+ err = test_rdesc_fixup(hid_skel, uhid_fd, sysfs_fd);
+ ASSERT_OK(err, "hid_rdesc_fixup");
+
cleanup:
hid__destroy(hid_skel);
destroy(uhid_fd);
diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
index f28bb6007875..f7a64c637782 100644
--- a/tools/testing/selftests/bpf/progs/hid.c
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -18,3 +18,51 @@ int hid_first_event(struct hid_bpf_ctx *ctx)

return 0;
}
+
+static __u8 rdesc[] = {
+ 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */
+ 0x09, 0x32, /* USAGE (Z) */
+ 0x95, 0x01, /* REPORT_COUNT (1) */
+ 0x81, 0x06, /* INPUT (Data,Var,Rel) */
+
+ 0x06, 0x00, 0xff, /* Usage Page (Vendor Defined Page 1) */
+ 0x19, 0x01, /* USAGE_MINIMUM (1) */
+ 0x29, 0x03, /* USAGE_MAXIMUM (3) */
+ 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
+ 0x25, 0x01, /* LOGICAL_MAXIMUM (1) */
+ 0x95, 0x03, /* REPORT_COUNT (3) */
+ 0x75, 0x01, /* REPORT_SIZE (1) */
+ 0x91, 0x02, /* Output (Data,Var,Abs) */
+ 0x95, 0x01, /* REPORT_COUNT (1) */
+ 0x75, 0x05, /* REPORT_SIZE (5) */
+ 0x91, 0x01, /* Output (Cnst,Var,Abs) */
+
+ 0x06, 0x00, 0xff, /* Usage Page (Vendor Defined Page 1) */
+ 0x19, 0x06, /* USAGE_MINIMUM (6) */
+ 0x29, 0x08, /* USAGE_MAXIMUM (8) */
+ 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
+ 0x25, 0x01, /* LOGICAL_MAXIMUM (1) */
+ 0x95, 0x03, /* REPORT_COUNT (3) */
+ 0x75, 0x01, /* REPORT_SIZE (1) */
+ 0xb1, 0x02, /* Feature (Data,Var,Abs) */
+ 0x95, 0x01, /* REPORT_COUNT (1) */
+ 0x75, 0x05, /* REPORT_SIZE (5) */
+ 0x91, 0x01, /* Output (Cnst,Var,Abs) */
+
+ 0xc0, /* END_COLLECTION */
+ 0xc0, /* END_COLLECTION */
+};
+
+SEC("hid/rdesc_fixup")
+int hid_rdesc_fixup(struct hid_bpf_ctx *ctx)
+{
+ callback2_check = ctx->u.rdesc.data[4];
+
+ /* insert rdesc at offset 52 */
+ __builtin_memcpy(&ctx->u.rdesc.data[52], rdesc, sizeof(rdesc));
+ ctx->u.rdesc.size = sizeof(rdesc) + 52;
+
+ ctx->u.rdesc.data[4] = 0x42;
+
+ return 0;
+}
--
2.35.1

2022-02-24 13:31:44

by Benjamin Tissoires

Subject: [PATCH bpf-next v1 6/6] HID: bpf: add bpf_hid_raw_request helper function

When we are in a user_event context, we can talk to the device to fetch
or set features/outputs/inputs reports.
Add a bpf helper to do so. This helper is thus only available to
user_events, because calling this function while in IRQ context (any
other BPF type) is forbidden.

Signed-off-by: Benjamin Tissoires <[email protected]>
---
drivers/hid/hid-bpf.c | 63 +++++++++++++++++
drivers/hid/hid-core.c | 3 +-
include/linux/bpf-hid.h | 2 +
include/linux/hid.h | 1 +
include/uapi/linux/bpf.h | 8 +++
kernel/bpf/hid.c | 26 +++++++
tools/include/uapi/linux/bpf.h | 8 +++
tools/testing/selftests/bpf/prog_tests/hid.c | 71 +++++++++++++++++++-
tools/testing/selftests/bpf/progs/hid.c | 57 ++++++++++++++++
9 files changed, 236 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-bpf.c b/drivers/hid/hid-bpf.c
index d775bda9d28d..180941061e53 100644
--- a/drivers/hid/hid-bpf.c
+++ b/drivers/hid/hid-bpf.c
@@ -138,6 +138,68 @@ int hid_bpf_set_data(struct hid_device *hdev, u8 *buf, u64 offset, u8 n, u32 dat
return 0;
}

+int hid_bpf_raw_request(struct hid_device *hdev, u8 *buf, size_t size,
+ u8 rtype, u8 reqtype)
+{
+ struct hid_report *report;
+ struct hid_report_enum *report_enum;
+ u8 *dma_data;
+ u32 report_len;
+ int ret;
+
+ /* check arguments */
+ switch (rtype) {
+ case HID_INPUT_REPORT:
+ case HID_OUTPUT_REPORT:
+ case HID_FEATURE_REPORT:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (reqtype) {
+ case HID_REQ_GET_REPORT:
+ case HID_REQ_GET_IDLE:
+ case HID_REQ_GET_PROTOCOL:
+ case HID_REQ_SET_REPORT:
+ case HID_REQ_SET_IDLE:
+ case HID_REQ_SET_PROTOCOL:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (size < 1)
+ return -EINVAL;
+
+ report_enum = hdev->report_enum + rtype;
+ report = hid_get_report(report_enum, buf);
+ if (!report)
+ return -EINVAL;
+
+ report_len = hid_report_len(report);
+
+ if (size > report_len)
+ size = report_len;
+
+ dma_data = kmemdup(buf, size, GFP_KERNEL);
+ if (!dma_data)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev,
+ dma_data[0],
+ dma_data,
+ size,
+ rtype,
+ reqtype);
+
+ if (ret > 0)
+ memcpy(buf, dma_data, ret);
+
+ kfree(dma_data);
+ return ret;
+}
+
static int hid_bpf_run_progs(struct hid_device *hdev, enum bpf_hid_attach_type type,
struct hid_bpf_ctx *ctx, u8 *data, int size)
{
@@ -251,6 +313,7 @@ int __init hid_bpf_module_init(void)
.array_detached = hid_bpf_array_detached,
.hid_get_data = hid_bpf_get_data,
.hid_set_data = hid_bpf_set_data,
+ .hid_raw_request = hid_bpf_raw_request,
};

bpf_hid_set_hooks(&hooks);
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index d3f4499ee4cd..d0e015986e17 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1686,8 +1686,7 @@ int hid_set_field(struct hid_field *field, unsigned offset, __s32 value)
}
EXPORT_SYMBOL_GPL(hid_set_field);

-static struct hid_report *hid_get_report(struct hid_report_enum *report_enum,
- const u8 *data)
+struct hid_report *hid_get_report(struct hid_report_enum *report_enum, const u8 *data)
{
struct hid_report *report;
unsigned int n = 0; /* Normally report number is 0 */
diff --git a/include/linux/bpf-hid.h b/include/linux/bpf-hid.h
index 00ac4555aa5b..05d88b48c315 100644
--- a/include/linux/bpf-hid.h
+++ b/include/linux/bpf-hid.h
@@ -77,6 +77,8 @@ struct bpf_hid_hooks {
void (*array_detached)(struct hid_device *hdev, enum bpf_hid_attach_type type);
int (*hid_get_data)(struct hid_device *hdev, u8 *buf, u64 offset, u8 size);
int (*hid_set_data)(struct hid_device *hdev, u8 *buf, u64 offset, u8 size, u32 data);
+ int (*hid_raw_request)(struct hid_device *hdev, u8 *buf, size_t size,
+ u8 rtype, u8 reqtype);
};

#ifdef CONFIG_BPF
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 7454e844324c..b2698df31e5b 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -946,6 +946,7 @@ __u32 hid_field_extract(const struct hid_device *hid, __u8 *report,
unsigned offset, unsigned n);
void implement(const struct hid_device *hid, u8 *report, unsigned int offset, unsigned int n,
u32 value);
+struct hid_report *hid_get_report(struct hid_report_enum *report_enum, const u8 *data);

#ifdef CONFIG_PM
int hid_driver_suspend(struct hid_device *hdev, pm_message_t state);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a374cc4aade6..058095d9961d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5105,6 +5105,13 @@ union bpf_attr {
* ctx->event.data field
* Return
* 0 on success, a negative error on failure.
+ *
+ * int bpf_hid_raw_request(void *ctx, void *buf, u64 size, u8 rtype, u8 reqtype)
+ * Description
+ * communicate with the HID device
+ * Return
+ * 0 on success.
+ * negative value on error.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -5301,6 +5308,7 @@ union bpf_attr {
FN(copy_from_user_task), \
FN(hid_get_data), \
FN(hid_set_data), \
+ FN(hid_raw_request), \
/* */

/* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c
index 3714413e1eb6..9dfb66f9b1b6 100644
--- a/kernel/bpf/hid.c
+++ b/kernel/bpf/hid.c
@@ -107,6 +107,28 @@ static const struct bpf_func_proto bpf_hid_set_data_proto = {
.arg4_type = ARG_ANYTHING,
};

+BPF_CALL_5(bpf_hid_raw_request, void*, ctx, void*, buf, u64, size,
+ u8, rtype, u8, reqtype)
+{
+ struct hid_bpf_ctx *bpf_ctx = ctx;
+
+ if (!hid_hooks.hid_raw_request)
+ return -EOPNOTSUPP;
+
+ return hid_hooks.hid_raw_request(bpf_ctx->hdev, buf, size, rtype, reqtype);
+}
+
+static const struct bpf_func_proto bpf_hid_raw_request_proto = {
+ .func = bpf_hid_raw_request,
+ .gpl_only = true, /* hid_raw_request is EXPORT_SYMBOL_GPL */
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_PTR_TO_MEM,
+ .arg3_type = ARG_CONST_SIZE_OR_ZERO,
+ .arg4_type = ARG_ANYTHING,
+ .arg5_type = ARG_ANYTHING,
+};
+
static const struct bpf_func_proto *
hid_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
@@ -115,6 +137,10 @@ hid_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_hid_get_data_proto;
case BPF_FUNC_hid_set_data:
return &bpf_hid_set_data_proto;
+ case BPF_FUNC_hid_raw_request:
+ if (prog->expected_attach_type != BPF_HID_DEVICE_EVENT)
+ return &bpf_hid_raw_request_proto;
+ return NULL;
default:
return bpf_base_func_proto(func_id);
}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a374cc4aade6..058095d9961d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5105,6 +5105,13 @@ union bpf_attr {
* ctx->event.data field
* Return
* 0 on success, a negative error on failure.
+ *
+ * int bpf_hid_raw_request(void *ctx, void *buf, u64 size, u8 rtype, u8 reqtype)
+ * Description
+ * communicate with the HID device
+ * Return
+ * 0 on success.
+ * negative value on error.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -5301,6 +5308,7 @@ union bpf_attr {
FN(copy_from_user_task), \
FN(hid_get_data), \
FN(hid_set_data), \
+ FN(hid_raw_request), \
/* */

/* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index b0cf615b0d0f..3dbad78ec121 100644
--- a/tools/testing/selftests/bpf/prog_tests/hid.c
+++ b/tools/testing/selftests/bpf/prog_tests/hid.c
@@ -67,6 +67,8 @@ static unsigned char rdesc[] = {
0xc0, /* END_COLLECTION */
};

+static u8 feature_data[] = { 1, 2 };
+
static pthread_mutex_t uhid_started_mtx = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t uhid_started = PTHREAD_COND_INITIALIZER;

@@ -126,7 +128,7 @@ static void destroy(int fd)

static int event(int fd)
{
- struct uhid_event ev;
+ struct uhid_event ev, answer;
ssize_t ret;

memset(&ev, 0, sizeof(ev));
@@ -143,6 +145,8 @@ static int event(int fd)
return -EFAULT;
}

+ memset(&answer, 0, sizeof(answer));
+
switch (ev.type) {
case UHID_START:
pthread_mutex_lock(&uhid_started_mtx);
@@ -167,6 +171,15 @@ static int event(int fd)
break;
case UHID_GET_REPORT:
fprintf(stderr, "UHID_GET_REPORT from uhid-dev\n");
+
+ answer.type = UHID_GET_REPORT_REPLY;
+ answer.u.get_report_reply.id = ev.u.get_report.id;
+ answer.u.get_report_reply.err = ev.u.get_report.rnum == 1 ? 0 : -EIO;
+ answer.u.get_report_reply.size = sizeof(feature_data);
+ memcpy(answer.u.get_report_reply.data, feature_data, sizeof(feature_data));
+
+ uhid_write(fd, &answer);
+
break;
case UHID_SET_REPORT:
fprintf(stderr, "UHID_SET_REPORT from uhid-dev\n");
@@ -493,6 +506,59 @@ static int test_hid_user_call(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
return ret;
}

+/*
+ * Attach hid_user_raw_request to the given uhid device,
+ * call the bpf program from userspace
+ * check that the program is called and does the expected.
+ */
+static int test_hid_user_raw_request_call(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
+{
+ int err, prog_fd;
+ u8 buf[10] = {0};
+ int ret = -1;
+
+ LIBBPF_OPTS(bpf_test_run_opts, run_attrs,
+ .repeat = 1,
+ .ctx_in = &sysfs_fd,
+ .ctx_size_in = sizeof(sysfs_fd),
+ .data_in = buf,
+ .data_size_in = sizeof(buf),
+ .data_out = buf,
+ .data_size_out = sizeof(buf),
+ );
+
+ /* attach hid_user_raw_request program */
+ hid_skel->links.hid_user_raw_request =
+ bpf_program__attach_hid(hid_skel->progs.hid_user_raw_request, sysfs_fd);
+ if (!ASSERT_OK_PTR(hid_skel->links.hid_user_raw_request,
+ "attach_hid(hid_user_raw_request)"))
+ return PTR_ERR(hid_skel->links.hid_user_raw_request);
+
+ buf[0] = 2; /* HID_FEATURE_REPORT */
+ buf[1] = 1; /* HID_REQ_GET_REPORT */
+ buf[2] = 1; /* report ID */
+
+ prog_fd = bpf_program__fd(hid_skel->progs.hid_user_raw_request);
+
+ err = bpf_prog_test_run_opts(prog_fd, &run_attrs);
+ if (!ASSERT_EQ(err, 0, "bpf_prog_test_run_xattr"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(run_attrs.retval, 2, "bpf_prog_test_run_xattr_retval"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[3], 2, "hid_user_raw_request_check_in"))
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+
+ hid__detach(hid_skel);
+
+ return ret;
+}
+
/*
* Attach hid_rdesc_fixup to the given uhid device,
* retrieve and open the matching hidraw node,
@@ -603,6 +669,9 @@ void serial_test_hid_bpf(void)
err = test_hid_user_call(hid_skel, uhid_fd, sysfs_fd);
ASSERT_OK(err, "hid_user");

+ err = test_hid_user_raw_request_call(hid_skel, uhid_fd, sysfs_fd);
+ ASSERT_OK(err, "hid_user_raw_request");
+
err = test_rdesc_fixup(hid_skel, uhid_fd, sysfs_fd);
ASSERT_OK(err, "hid_rdesc_fixup");

diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
index b2db809b3367..d49eb9e0e745 100644
--- a/tools/testing/selftests/bpf/progs/hid.c
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -9,6 +9,11 @@ char _license[] SEC("license") = "GPL";
__u64 callback_check = 52;
__u64 callback2_check = 52;

+struct {
+ __uint(type, BPF_MAP_TYPE_RINGBUF);
+ __uint(max_entries, 4096 * 64);
+} ringbuf SEC(".maps");
+
SEC("hid/device_event")
int hid_first_event(struct hid_bpf_ctx *ctx)
{
@@ -90,3 +95,55 @@ int hid_user(struct hid_bpf_ctx *ctx)

return 0;
}
+
+SEC("hid/user_event")
+int hid_user_raw_request(struct hid_bpf_ctx *ctx)
+{
+ const unsigned int buflen = 256;
+ const unsigned int _buflen = buflen * sizeof(__u8);
+ __u8 *buf;
+ int ret;
+ __u32 size;
+ __u8 rtype, reqtype;
+
+ buf = bpf_ringbuf_reserve(&ringbuf, _buflen, 0);
+ if (!buf)
+ return -12; /* -ENOMEM */
+
+ __builtin_memcpy(buf, ctx->u.user.data, _buflen);
+
+ /*
+ * build up a custom API for our needs:
+ * offset 0, size 1: report type
+ * offset 1, size 1: request type
+ * offset 2+: data
+ */
+ rtype = buf[0];
+ reqtype = buf[1];
+ size = ctx->u.user.size - 2;
+
+ if (size < _buflen - 2) {
+ ret = bpf_hid_raw_request(ctx,
+ &buf[2],
+ size,
+ rtype,
+ reqtype);
+ if (ret < 0)
+ goto discard;
+ } else {
+ ret = -7; /* -E2BIG */
+ goto discard;
+ }
+
+ __builtin_memcpy(&ctx->u.user.data[2], &buf[2], _buflen - 2);
+
+ ctx->u.user.size = ret + 2;
+ ctx->u.user.retval = ret;
+
+ ret = 0;
+
+ discard:
+ bpf_ringbuf_discard(buf, 0);
+
+ return ret;
+}
--
2.35.1

2022-02-24 15:03:57

by Greg KH

Subject: Re: [PATCH bpf-next v1 1/6] HID: initial BPF implementation

On Thu, Feb 24, 2022 at 12:08:23PM +0100, Benjamin Tissoires wrote:
> index 000000000000..243ac45a253f
> --- /dev/null
> +++ b/include/uapi/linux/bpf_hid.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */
> +
> +/*
> + * HID BPF public headers
> + *
> + * Copyright (c) 2021 Benjamin Tissoires
> + */
> +
> +#ifndef _UAPI__LINUX_BPF_HID_H__
> +#define _UAPI__LINUX_BPF_HID_H__
> +
> +#include <linux/types.h>
> +
> +#define HID_BPF_MAX_BUFFER_SIZE 16384 /* 16kb */
> +
> +struct hid_device;
> +
> +enum hid_bpf_event {
> + HID_BPF_UNDEF = 0,
> + HID_BPF_DEVICE_EVENT,
> +};
> +
> +/* type is HID_BPF_DEVICE_EVENT */
> +struct hid_bpf_ctx_device_event {
> + __u8 data[HID_BPF_MAX_BUFFER_SIZE];
> + unsigned long size;

That's not a valid type to cross the user/kernel boundry, shouldn't it
be "__u64"? But really, isn't __u32 enough here?

thanks,

greg k-h

2022-02-24 15:35:54

by Benjamin Tissoires

Subject: [PATCH bpf-next v1 5/6] HID: bpf: tests: rely on uhid event to know if a test device is ready

We need this for 2 reasons:
- first we remove the ugly sleeps
- then when we try to communicate with the device, we need to have another
thread that handles that communication and simulate a real device

Signed-off-by: Benjamin Tissoires <[email protected]>
---
tools/testing/selftests/bpf/prog_tests/hid.c | 126 ++++++++++++++++++-
1 file changed, 120 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index d297a571e910..b0cf615b0d0f 100644
--- a/tools/testing/selftests/bpf/prog_tests/hid.c
+++ b/tools/testing/selftests/bpf/prog_tests/hid.c
@@ -67,6 +67,12 @@ static unsigned char rdesc[] = {
0xc0, /* END_COLLECTION */
};

+static pthread_mutex_t uhid_started_mtx = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t uhid_started = PTHREAD_COND_INITIALIZER;
+
+/* no need to protect uhid_stopped, only one thread accesses it */
+static bool uhid_stopped;
+
static int uhid_write(int fd, const struct uhid_event *ev)
{
ssize_t ret;
@@ -118,6 +124,104 @@ static void destroy(int fd)
uhid_write(fd, &ev);
}

+static int event(int fd)
+{
+ struct uhid_event ev;
+ ssize_t ret;
+
+ memset(&ev, 0, sizeof(ev));
+ ret = read(fd, &ev, sizeof(ev));
+ if (ret == 0) {
+ fprintf(stderr, "Read HUP on uhid-cdev\n");
+ return -EFAULT;
+ } else if (ret < 0) {
+ fprintf(stderr, "Cannot read uhid-cdev: %m\n");
+ return -errno;
+ } else if (ret != sizeof(ev)) {
+ fprintf(stderr, "Invalid size read from uhid-dev: %zd != %zu\n",
+ ret, sizeof(ev));
+ return -EFAULT;
+ }
+
+ switch (ev.type) {
+ case UHID_START:
+ pthread_mutex_lock(&uhid_started_mtx);
+ pthread_cond_signal(&uhid_started);
+ pthread_mutex_unlock(&uhid_started_mtx);
+
+ fprintf(stderr, "UHID_START from uhid-dev\n");
+ break;
+ case UHID_STOP:
+ uhid_stopped = true;
+
+ fprintf(stderr, "UHID_STOP from uhid-dev\n");
+ break;
+ case UHID_OPEN:
+ fprintf(stderr, "UHID_OPEN from uhid-dev\n");
+ break;
+ case UHID_CLOSE:
+ fprintf(stderr, "UHID_CLOSE from uhid-dev\n");
+ break;
+ case UHID_OUTPUT:
+ fprintf(stderr, "UHID_OUTPUT from uhid-dev\n");
+ break;
+ case UHID_GET_REPORT:
+ fprintf(stderr, "UHID_GET_REPORT from uhid-dev\n");
+ break;
+ case UHID_SET_REPORT:
+ fprintf(stderr, "UHID_SET_REPORT from uhid-dev\n");
+ break;
+ default:
+ fprintf(stderr, "Invalid event from uhid-dev: %u\n", ev.type);
+ }
+
+ return 0;
+}
+
+static void *read_uhid_events_thread(void *arg)
+{
+ int fd = *(int *)arg;
+ struct pollfd pfds[1];
+ int ret = 0;
+
+ pfds[0].fd = fd;
+ pfds[0].events = POLLIN;
+
+ uhid_stopped = false;
+
+ while (!uhid_stopped) {
+ ret = poll(pfds, 1, 100);
+ if (ret < 0) {
+ fprintf(stderr, "Cannot poll for fds: %m\n");
+ break;
+ }
+ if (pfds[0].revents & POLLIN) {
+ ret = event(fd);
+ if (ret)
+ break;
+ }
+ }
+
+ return (void *)(long)ret;
+}
+
+static int uhid_start_listener(pthread_t *tid, int uhid_fd)
+{
+ int fd = uhid_fd;
+
+ pthread_mutex_lock(&uhid_started_mtx);
+ if (CHECK_FAIL(pthread_create(tid, NULL, read_uhid_events_thread,
+ (void *)&fd))) {
+ pthread_mutex_unlock(&uhid_started_mtx);
+ close(fd);
+ return -EIO;
+ }
+ pthread_cond_wait(&uhid_started, &uhid_started_mtx);
+ pthread_mutex_unlock(&uhid_started_mtx);
+
+ return 0;
+}
+
static int send_event(int fd, u8 *buf, size_t size)
{
struct uhid_event ev;
@@ -399,7 +503,9 @@ static int test_rdesc_fixup(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
struct hidraw_report_descriptor rpt_desc = {0};
int err, desc_size, hidraw_ino, hidraw_fd = -1;
char hidraw_path[64] = {0};
+ void *uhid_err;
int ret = -1;
+ pthread_t tid;

/* attach the program */
hid_skel->links.hid_rdesc_fixup =
@@ -408,9 +514,8 @@ static int test_rdesc_fixup(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
"attach_hid(hid_rdesc_fixup)"))
return PTR_ERR(hid_skel->links.hid_rdesc_fixup);

- /* give a little bit of time for the device to appear */
- /* TODO: check on uhid events */
- usleep(1000);
+ err = uhid_start_listener(&tid, uhid_fd);
+ ASSERT_OK(err, "uhid_start_listener");

hidraw_ino = get_hidraw(hid_skel->links.hid_rdesc_fixup);
if (!ASSERT_GE(hidraw_ino, 0, "get_hidraw"))
@@ -451,6 +556,10 @@ static int test_rdesc_fixup(struct hid *hid_skel, int uhid_fd, int sysfs_fd)

hid__detach(hid_skel);

+ pthread_join(tid, &uhid_err);
+ err = (int)(long)uhid_err;
+ CHECK_FAIL(err);
+
return ret;
}

@@ -458,7 +567,9 @@ void serial_test_hid_bpf(void)
{
struct hid *hid_skel = NULL;
int err, uhid_fd, sysfs_fd;
+ void *uhid_err;
time_t t;
+ pthread_t tid;
int rand_nb;

/* initialize random number generator */
@@ -470,9 +581,8 @@ void serial_test_hid_bpf(void)
if (!ASSERT_GE(uhid_fd, 0, "setup uhid"))
return;

- /* give a little bit of time for the device to appear */
- /* TODO: check on uhid events */
- usleep(1000);
+ err = uhid_start_listener(&tid, uhid_fd);
+ ASSERT_OK(err, "uhid_start_listener");

/* locate the uevent file of the created device */
sysfs_fd = get_sysfs_fd(rand_nb);
@@ -499,4 +609,8 @@ void serial_test_hid_bpf(void)
cleanup:
hid__destroy(hid_skel);
destroy(uhid_fd);
+
+ pthread_join(tid, &uhid_err);
+ err = (int)(long)uhid_err;
+ CHECK_FAIL(err);
}
--
2.35.1

2022-02-24 16:07:19

by Benjamin Tissoires

Subject: [PATCH bpf-next v1 4/6] HID: bpf: add new BPF type to trigger commands from userspace

Given that we can not call bpf_hid_raw_request() from within an IRQ,
userspace needs to have a way to communicate with the device when
it needs.

Implement a new type that the caller can run at will without being in
an IRQ context.

Signed-off-by: Benjamin Tissoires <[email protected]>
---
include/linux/bpf-hid.h | 3 +
include/uapi/linux/bpf.h | 1 +
include/uapi/linux/bpf_hid.h | 9 ++
kernel/bpf/hid.c | 117 +++++++++++++++++++
kernel/bpf/syscall.c | 2 +
tools/include/uapi/linux/bpf.h | 1 +
tools/lib/bpf/libbpf.c | 1 +
tools/testing/selftests/bpf/prog_tests/hid.c | 56 +++++++++
tools/testing/selftests/bpf/progs/hid.c | 10 ++
9 files changed, 200 insertions(+)

diff --git a/include/linux/bpf-hid.h b/include/linux/bpf-hid.h
index 07cbd5cf595c..00ac4555aa5b 100644
--- a/include/linux/bpf-hid.h
+++ b/include/linux/bpf-hid.h
@@ -15,6 +15,7 @@ enum bpf_hid_attach_type {
BPF_HID_ATTACH_INVALID = -1,
BPF_HID_ATTACH_DEVICE_EVENT = 0,
BPF_HID_ATTACH_RDESC_FIXUP,
+ BPF_HID_ATTACH_USER_EVENT,
MAX_BPF_HID_ATTACH_TYPE
};

@@ -34,6 +35,8 @@ to_bpf_hid_attach_type(enum bpf_attach_type attach_type)
return BPF_HID_ATTACH_DEVICE_EVENT;
case BPF_HID_RDESC_FIXUP:
return BPF_HID_ATTACH_RDESC_FIXUP;
+ case BPF_HID_USER_EVENT:
+ return BPF_HID_ATTACH_USER_EVENT;
default:
return BPF_HID_ATTACH_INVALID;
}
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0571d9b954c9..a374cc4aade6 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1000,6 +1000,7 @@ enum bpf_attach_type {
BPF_PERF_EVENT,
BPF_HID_DEVICE_EVENT,
BPF_HID_RDESC_FIXUP,
+ BPF_HID_USER_EVENT,
__MAX_BPF_ATTACH_TYPE
};

diff --git a/include/uapi/linux/bpf_hid.h b/include/uapi/linux/bpf_hid.h
index c0801d7174c3..7a263568e132 100644
--- a/include/uapi/linux/bpf_hid.h
+++ b/include/uapi/linux/bpf_hid.h
@@ -19,6 +19,7 @@ enum hid_bpf_event {
HID_BPF_UNDEF = 0,
HID_BPF_DEVICE_EVENT,
HID_BPF_RDESC_FIXUP,
+ HID_BPF_USER_EVENT,
};

/* type is HID_BPF_DEVICE_EVENT */
@@ -33,6 +34,13 @@ struct hid_bpf_ctx_rdesc_fixup {
unsigned long size;
};

+/* type is HID_BPF_USER_EVENT */
+struct hid_bpf_ctx_user_event {
+ __u8 data[HID_BPF_MAX_BUFFER_SIZE];
+ unsigned long size;
+ int retval;
+};
+
struct hid_bpf_ctx {
enum hid_bpf_event type;
struct hid_device *hdev;
@@ -40,6 +48,7 @@ struct hid_bpf_ctx {
union {
struct hid_bpf_ctx_device_event device;
struct hid_bpf_ctx_rdesc_fixup rdesc;
+ struct hid_bpf_ctx_user_event user;
} u;
};

diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c
index 9eb7bd6ac6c8..3714413e1eb6 100644
--- a/kernel/bpf/hid.c
+++ b/kernel/bpf/hid.c
@@ -52,6 +52,9 @@ BPF_CALL_3(bpf_hid_get_data, void*, ctx, u64, offset, u8, n)
case HID_BPF_RDESC_FIXUP:
buf = bpf_ctx->u.rdesc.data;
break;
+ case HID_BPF_USER_EVENT:
+ buf = bpf_ctx->u.user.data;
+ break;
default:
return -EOPNOTSUPP;
}
@@ -83,6 +86,9 @@ BPF_CALL_4(bpf_hid_set_data, void*, ctx, u64, offset, u8, n, u32, data)
case HID_BPF_RDESC_FIXUP:
buf = bpf_ctx->u.rdesc.data;
break;
+ case HID_BPF_USER_EVENT:
+ buf = bpf_ctx->u.user.data;
+ break;
default:
return -EOPNOTSUPP;
}
@@ -385,6 +391,8 @@ static int bpf_hid_max_progs(enum bpf_hid_attach_type type)
return 64;
case BPF_HID_ATTACH_RDESC_FIXUP:
return 1;
+ case BPF_HID_ATTACH_USER_EVENT:
+ return 64;
default:
return 0;
}
@@ -479,7 +487,116 @@ int bpf_hid_link_create(const union bpf_attr *attr, struct bpf_prog *prog)
return bpf_link_settle(&link_primer);
}

+static int hid_bpf_prog_test_run(struct bpf_prog *prog,
+ const union bpf_attr *attr,
+ union bpf_attr __user *uattr)
+{
+ struct hid_device *hdev = NULL;
+ struct bpf_prog_array *progs;
+ struct hid_bpf_ctx *ctx = NULL;
+ bool valid_prog = false;
+ int i;
+ int target_fd, ret;
+ void __user *data_out = u64_to_user_ptr(attr->test.data_out);
+ void __user *data_in = u64_to_user_ptr(attr->test.data_in);
+ u32 user_size = attr->test.data_size_in;
+
+ if (!hid_hooks.hdev_from_fd)
+ return -EOPNOTSUPP;
+
+ if (attr->test.ctx_size_in != sizeof(int))
+ return -EINVAL;
+
+ if (copy_from_user(&target_fd, (void *)attr->test.ctx_in, attr->test.ctx_size_in))
+ return -EFAULT;
+
+ hdev = hid_hooks.hdev_from_fd(target_fd);
+ if (IS_ERR(hdev))
+ return PTR_ERR(hdev);
+
+ ret = mutex_lock_interruptible(&bpf_hid_mutex);
+ if (ret)
+ return ret;
+
+ /* check if the given program is of correct type and registered */
+ progs = rcu_dereference_protected(hdev->bpf.run_array[BPF_HID_ATTACH_USER_EVENT],
+ lockdep_is_held(&bpf_hid_mutex));
+ if (!progs) {
+ ret = -EFAULT;
+ goto unlock;
+ }
+
+ for (i = 0; i < bpf_prog_array_length(progs); i++) {
+ if (progs->items[i].prog == prog) {
+ valid_prog = true;
+ break;
+ }
+ }
+
+ if (!valid_prog) {
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+ if (!ctx) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ ctx->hdev = hdev;
+ ctx->type = HID_BPF_USER_EVENT;
+
+ /* copy data_in from userspace */
+ if (user_size) {
+ if (user_size > HID_BPF_MAX_BUFFER_SIZE)
+ user_size = HID_BPF_MAX_BUFFER_SIZE;
+
+ if (copy_from_user(ctx->u.user.data, data_in, user_size)) {
+ ret = -EFAULT;
+ goto unlock;
+ }
+
+ ctx->u.user.size = user_size;
+ }
+
+ migrate_disable();
+
+ ret = bpf_prog_run(prog, ctx);
+
+ migrate_enable();
+
+ user_size = attr->test.data_size_out;
+
+ if (user_size && data_out) {
+ if (user_size > ctx->u.user.size)
+ user_size = ctx->u.user.size;
+
+ if (copy_to_user(data_out, ctx->u.user.data, user_size)) {
+ ret = -EFAULT;
+ goto unlock;
+ }
+
+ if (copy_to_user(&uattr->test.data_size_out, &user_size, sizeof(user_size))) {
+ ret = -EFAULT;
+ goto unlock;
+ }
+ }
+
+ if (copy_to_user(&uattr->test.retval, &ctx->u.user.retval, sizeof(ctx->u.user.retval))) {
+ ret = -EFAULT;
+ goto unlock;
+ }
+
+unlock:
+ kfree(ctx);
+
+ mutex_unlock(&bpf_hid_mutex);
+ return ret;
+}
+
const struct bpf_prog_ops hid_prog_ops = {
+ .test_run = hid_bpf_prog_test_run,
};

int bpf_hid_init(struct hid_device *hdev)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 62889cc71a02..0a6d08dabe59 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3192,6 +3192,7 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
return BPF_PROG_TYPE_XDP;
case BPF_HID_DEVICE_EVENT:
case BPF_HID_RDESC_FIXUP:
+ case BPF_HID_USER_EVENT:
return BPF_PROG_TYPE_HID;
default:
return BPF_PROG_TYPE_UNSPEC;
@@ -3338,6 +3339,7 @@ static int bpf_prog_query(const union bpf_attr *attr,
return sock_map_bpf_prog_query(attr, uattr);
case BPF_HID_DEVICE_EVENT:
case BPF_HID_RDESC_FIXUP:
+ case BPF_HID_USER_EVENT:
return bpf_hid_prog_query(attr, uattr);
default:
return -EINVAL;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 0571d9b954c9..a374cc4aade6 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1000,6 +1000,7 @@ enum bpf_attach_type {
BPF_PERF_EVENT,
BPF_HID_DEVICE_EVENT,
BPF_HID_RDESC_FIXUP,
+ BPF_HID_USER_EVENT,
__MAX_BPF_ATTACH_TYPE
};

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b7af873116fb..290864d2f865 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8678,6 +8678,7 @@ static const struct bpf_sec_def section_defs[] = {
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_SLOPPY_PFX),
SEC_DEF("hid/rdesc_fixup", HID, BPF_HID_RDESC_FIXUP, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX),
+ SEC_DEF("hid/user_event", HID, BPF_HID_USER_EVENT, SEC_ATTACHABLE_OPT | SEC_SLOPPY_PFX),
};

#define MAX_TYPE_NAME_SIZE 32
diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index 7d4f740a0a08..d297a571e910 100644
--- a/tools/testing/selftests/bpf/prog_tests/hid.c
+++ b/tools/testing/selftests/bpf/prog_tests/hid.c
@@ -336,6 +336,59 @@ static int test_hid_set_get_data(struct hid *hid_skel, int uhid_fd, int sysfs_fd
return ret;
}

+/*
+ * Attach hid_user to the given uhid device,
+ * call the bpf program from userspace
+ * check that the program is called and does the expected.
+ */
+static int test_hid_user_call(struct hid *hid_skel, int uhid_fd, int sysfs_fd)
+{
+ int err, prog_fd;
+ u8 buf[10] = {0};
+ int ret = -1;
+
+ LIBBPF_OPTS(bpf_test_run_opts, run_attrs,
+ .repeat = 1,
+ .ctx_in = &sysfs_fd,
+ .ctx_size_in = sizeof(sysfs_fd),
+ .data_in = buf,
+ .data_size_in = sizeof(buf),
+ .data_out = buf,
+ .data_size_out = sizeof(buf),
+ );
+
+ /* attach hid_user program */
+ hid_skel->links.hid_user = bpf_program__attach_hid(hid_skel->progs.hid_user, sysfs_fd);
+ if (!ASSERT_OK_PTR(hid_skel->links.hid_user,
+ "attach_hid(hid_user)"))
+ return PTR_ERR(hid_skel->links.hid_user);
+
+ buf[0] = 39;
+
+ prog_fd = bpf_program__fd(hid_skel->progs.hid_user);
+
+ err = bpf_prog_test_run_opts(prog_fd, &run_attrs);
+ if (!ASSERT_EQ(err, 0, "bpf_prog_test_run_xattr"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(run_attrs.retval, 72, "bpf_prog_test_run_xattr_retval"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[1], 42, "hid_user_check_in"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[2], 4, "hid_user_check_static_out"))
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+
+ hid__detach(hid_skel);
+
+ return ret;
+}
+
/*
* Attach hid_rdesc_fixup to the given uhid device,
* retrieve and open the matching hidraw node,
@@ -437,6 +490,9 @@ void serial_test_hid_bpf(void)
err = test_hid_set_get_data(hid_skel, uhid_fd, sysfs_fd);
ASSERT_OK(err, "hid_set_get_data");

+ err = test_hid_user_call(hid_skel, uhid_fd, sysfs_fd);
+ ASSERT_OK(err, "hid_user");
+
err = test_rdesc_fixup(hid_skel, uhid_fd, sysfs_fd);
ASSERT_OK(err, "hid_rdesc_fixup");

diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
index 01d9c556a3a1..b2db809b3367 100644
--- a/tools/testing/selftests/bpf/progs/hid.c
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -80,3 +80,13 @@ int hid_set_get_data(struct hid_bpf_ctx *ctx)

return 0;
}
+
+SEC("hid/user_event")
+int hid_user(struct hid_bpf_ctx *ctx)
+{
+ ctx->u.user.data[1] = ctx->u.user.data[0] + 3;
+ ctx->u.user.data[2] = 4;
+ ctx->u.user.retval = 72;
+
+ return 0;
+}
--
2.35.1

2022-02-24 16:09:38

by Benjamin Tissoires

Subject: Re: [PATCH bpf-next v1 1/6] HID: initial BPF implementation

On Thu, Feb 24, 2022 at 12:41 PM Greg KH <[email protected]> wrote:
>
> On Thu, Feb 24, 2022 at 12:08:23PM +0100, Benjamin Tissoires wrote:
> > index 000000000000..243ac45a253f
> > --- /dev/null
> > +++ b/include/uapi/linux/bpf_hid.h
> > @@ -0,0 +1,39 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */
> > +
> > +/*
> > + * HID BPF public headers
> > + *
> > + * Copyright (c) 2021 Benjamin Tissoires
> > + */
> > +
> > +#ifndef _UAPI__LINUX_BPF_HID_H__
> > +#define _UAPI__LINUX_BPF_HID_H__
> > +
> > +#include <linux/types.h>
> > +
> > +#define HID_BPF_MAX_BUFFER_SIZE 16384 /* 16kb */
> > +
> > +struct hid_device;
> > +
> > +enum hid_bpf_event {
> > + HID_BPF_UNDEF = 0,
> > + HID_BPF_DEVICE_EVENT,
> > +};
> > +
> > +/* type is HID_BPF_DEVICE_EVENT */
> > +struct hid_bpf_ctx_device_event {
> > + __u8 data[HID_BPF_MAX_BUFFER_SIZE];
> > + unsigned long size;
>
> That's not a valid type to cross the user/kernel boundry, shouldn't it
> be "__u64"? But really, isn't __u32 enough here?

thanks. Even __u16 should be enough, given that the upper bound is
16384. I'll amend it in v2.

Cheers,
Benjamin

>
> thanks,
>
> greg k-h
>

2022-02-24 16:26:53

by Benjamin Tissoires

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

Hi Greg,

Thanks for the quick answer :)

On Thu, Feb 24, 2022 at 12:31 PM Greg KH <[email protected]> wrote:
>
> On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> > Hi there,
> >
> > This series introduces support of eBPF for HID devices.
> >
> > I have several use cases where eBPF could be interesting for those
> > input devices:
> >
> > - simple fixup of report descriptor:
> >
> > In the HID tree, we have half of the drivers that are "simple" and
> > that just fix one key or one byte in the report descriptor.
> > Currently, for users of such devices, the process of fixing them
> > is long and painful.
> > With eBPF, we could externalize those fixups in one external repo,
> > ship various CoRe bpf programs and have those programs loaded at boot
> > time without having to install a new kernel (and wait 6 months for the
> > fix to land in the distro kernel)
>
> Why would a distro update such an external repo faster than they update
> the kernel? Many sane distros update their kernel faster than other
> packages already, how about fixing your distro? :)

Heh, I'm going to try to dodge the incoming rhel bullet :)

It's true that thanks to the work of the stable folks we don't have to
wait 6 months for a fix to come in. However, I think having a single
file to drop in a directory would be easier for development/testing
(and distribution of that file between developers/testers) than
requiring people to recompile their kernel.

Brain fart: is there any chance we could keep the validated bpf
programs in the kernel tree?

>
> I'm all for the idea of using ebpf for HID devices, but now we have to
> keep track of multiple packages to be in sync here. Is this making
> things harder overall?

Probably, and this is also maybe opening a can of worms. Vendors will
be able to say "use that bpf program for my HID device because the
firmware is bogus".

OTOH, as far as I understand, you can not load a BPF program in the
kernel that uses GPL-declared functions if your BPF program is not
GPL. Which means that if firmware vendors want to distribute blobs
through BPF, either it's GPL and they have to provide the sources, or
it's not happening.

I am not entirely clear on which plan I want to have for userspace.
I'd like to have libinput on board, but right now, Peter's stance is
"not in my garden" (and he has good reasons for it).
So my initial plan is to cook and hold the bpf programs in hid-tools,
which is the repo I am using for the regression tests on HID.

I plan on building a systemd intrinsic that would detect the HID
VID/PID and then load the various BPF programs associated with the
small fixes.
Note that everything can not be fixed through eBPF, simply because at
boot we don't always have the root partition mounted.

>
> > - Universal Stylus Interface (or any other new fancy feature that
> > requires a new kernel API)
> >
> > See [0].
> > Basically, USI pens are requiring a new kernel API because there are
> > some channels of communication our HID and input stack are not capable
> > of. Instead of using hidraw or creating new sysfs or ioctls, we can rely
> > on eBPF to have the kernel API controlled by the consumer and to not
> > impact the performances by waking up userspace every time there is an
> > event.
>
> How is userspace supposed to interact with these devices in a unified
> way then? This would allow random new interfaces to be created, one
> each for each device, and again, be a pain to track for a distro to keep
> in sync. And how are you going to keep the ebpf interface these
> provides in sync with the userspace program?

Right now, the idea we have is to export the USI specifics through
dbus. This has a couple of advantages: we are not tied to USI and can
"emulate" those parameters by storing them on disk instead of in the
pen, and this is easily accessible from all applications directly.

I am trying to push to have one implementation of that dbus service
with the Intel and ChromeOS folks so general linux doesn't have to
recreate it. But if you look at it, with hidraw nothing prevents
someone from writing such a library/daemon in its own world without
sharing it with anybody.

The main advantage of eBPF compared to hidraw is that you can analyse
the incoming event without waking userspace and only wake it up when
there is something noticeable.

In terms of random interfaces, yes, this is a good point. But the way
I see it is that we can provide one kernel API (eBPF for HID) which we
will maintain and not have to maintain forever a badly designed kernel
API for a specific device. Though also note that USI is a HID standard
(I think there is a second one), so AFAICT, the same bpf program
should be able to be generic enough to be cross vendor. So there will
be one provider only for USI.

>
> > - Surface Dial
> >
> > This device is a "puck" from Microsoft, basically a rotary dial with a
> > push button. The kernel already exports it as such but doesn't handle
> > the haptic feedback we can get out of it.
> > Furthermore, that device is not recognized by userspace and so it's a
> > nice paperwight in the end.
> >
> > With eBPF, we can morph that device into a mouse, and convert the dial
> > events into wheel events.
>
> Why can't we do this in the kernel today?

We can do this in the kernel, sure, but that means the kernel has to
make a choice.
Right now, the device is exported as a "rotary button". Userspace
should know what it is, and handle it properly.
Turns out that there are not so many developers who care about it, so
there is no implementation of it in userspace.

So the idea to morph it into a special mouse is interesting, but
suddenly we are lying to userspace about the device, and this can have
unanticipated consequences.

If we load a bpf program that morphs the device into a mouse, suddenly
the kernel is not the one responsible for that choice, but the user
is.

For instance, we could imagine a program that pops up a pie menu like
Windows does and enables/disables the haptic feedback based on what is
on screen.

With a kernel implementation, we need a driver with a config
parameter, a new haptic kernel API which is unlikely to be compatible
with the forcepad haptic API that Angela is working on :/

>
> > Also, we can set/unset the haptic feedback
> > from userspace. The convenient part of BPF makes it that the kernel
> > doesn't make any choice that would need to be reverted because that
> > specific userspace doesn't handle it properly or because that other
> > one expects it to be different.
>
> Again, what would the new api for the haptic device be? Who is going to
> mantain that on the userspace side? What library is going to use this?
> Is libinput going to now be responsible for interacting this way with
> the kernel?

In that particular case, I don't think the haptic API should be very
complex. On Windows, you only have a toggle: on/off.
And actually I'd love to see the haptic feedback enabled or disabled
based on the context: do you need one tick every 5 degrees? haptic
enabled, if not (smooth scrolling where every minimal step matters),
then haptic disabled.

Note that this is also entirely possible to be done in pure hidraw without BPF.

In terms of "who" that's up in the air. I am not using the device
enough to maintain such a tool (and definitively not skilled enough
for the UI part).

>
> > - 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)
> > With eBPF, we can intercept any HID command emitted to the device and
> > validate it or not.
>
> This I like.

Heh. It's a shame that it's the part I left out from the series :)

>
> > This also allows to sync the state between the userspace and the
> > kernel/bpf program because we can intercept any incoming command.
> >
> > - tracing
> >
> > The last usage I have in mind is tracing events and all the fun we can
> > do we BPF to summarize and analyze events.
> > Right now, tracing relies on hidraw. It works well except for a couple
> > of issues:
> > 1. if the driver doesn't export a hidraw node, we can't trace anything
> > (eBPF will be a "god-mode" there, so it might raise some eyebrows)
> > 2. hidraw doesn't catch the other process requests to the device, which
> > means that we have cases where we need to add printks to the kernel
> > to understand what is happening.
>
> Tracing is also nice, I like this too.
>
> Anyway, I like the idea, I'm just worried we are pushing complexity out
> into userspace which would make it "someone else's problem." The job of
> a kernel is to provide a way to abstract devices in a standard way. To
> force userspace to write a "new program per input device" would be a
> total mess and a step backwards.
>

Yeah, I completely understand the view. However, please keep in mind
that most of it (though not firewall and some corner cases of tracing)
is already possible to do through hidraw.
One other example of that is SDL. We got Sony involved to create a
nice driver for the DualSense controller (the PS5 one), but they
simply ignore it and use plain HID (through hidraw). They have the
advantage of this being cross-platform and can provide a consistent
experience across platforms. And as a result, in the kernel, we have
to hands up the handling of the device whenever somebody opens a
hidraw node for those devices (Steam is also doing the same FWIW).

Which reminds me that I also have another use-case: joystick
dead-zone. You can have a small filter that configures the dead zone
and doesn't even wake up userspace for those hardware glitches...

Anyway, IOW, I think the bpf approach will allow kernel-like
performances of hidraw applications, and I would be more inclined to
ask people to move their weird issue in userspace thanks to that.

And I am also open to any suggestions on how to better handle your remarks :)

Cheers,
Benjamin

2022-02-24 16:36:38

by Greg Kroah-Hartman

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> Hi there,
>
> This series introduces support of eBPF for HID devices.
>
> I have several use cases where eBPF could be interesting for those
> input devices:
>
> - simple fixup of report descriptor:
>
> In the HID tree, we have half of the drivers that are "simple" and
> that just fix one key or one byte in the report descriptor.
> Currently, for users of such devices, the process of fixing them
> is long and painful.
> With eBPF, we could externalize those fixups in one external repo,
> ship various CoRe bpf programs and have those programs loaded at boot
> time without having to install a new kernel (and wait 6 months for the
> fix to land in the distro kernel)

Why would a distro update such an external repo faster than they update
the kernel? Many sane distros update their kernel faster than other
packages already, how about fixing your distro? :)

I'm all for the idea of using ebpf for HID devices, but now we have to
keep track of multiple packages to be in sync here. Is this making
things harder overall?

> - Universal Stylus Interface (or any other new fancy feature that
> requires a new kernel API)
>
> See [0].
> Basically, USI pens are requiring a new kernel API because there are
> some channels of communication our HID and input stack are not capable
> of. Instead of using hidraw or creating new sysfs or ioctls, we can rely
> on eBPF to have the kernel API controlled by the consumer and to not
> impact the performances by waking up userspace every time there is an
> event.

How is userspace supposed to interact with these devices in a unified
way then? This would allow random new interfaces to be created, one
each for each device, and again, be a pain to track for a distro to keep
in sync. And how are you going to keep the ebpf interface these
provides in sync with the userspace program?

> - Surface Dial
>
> This device is a "puck" from Microsoft, basically a rotary dial with a
> push button. The kernel already exports it as such but doesn't handle
> the haptic feedback we can get out of it.
> Furthermore, that device is not recognized by userspace and so it's a
> nice paperwight in the end.
>
> With eBPF, we can morph that device into a mouse, and convert the dial
> events into wheel events.

Why can't we do this in the kernel today?

> Also, we can set/unset the haptic feedback
> from userspace. The convenient part of BPF makes it that the kernel
> doesn't make any choice that would need to be reverted because that
> specific userspace doesn't handle it properly or because that other
> one expects it to be different.

Again, what would the new api for the haptic device be? Who is going to
mantain that on the userspace side? What library is going to use this?
Is libinput going to now be responsible for interacting this way with
the kernel?

> - 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)
> With eBPF, we can intercept any HID command emitted to the device and
> validate it or not.

This I like.

> This also allows to sync the state between the userspace and the
> kernel/bpf program because we can intercept any incoming command.
>
> - tracing
>
> The last usage I have in mind is tracing events and all the fun we can
> do we BPF to summarize and analyze events.
> Right now, tracing relies on hidraw. It works well except for a couple
> of issues:
> 1. if the driver doesn't export a hidraw node, we can't trace anything
> (eBPF will be a "god-mode" there, so it might raise some eyebrows)
> 2. hidraw doesn't catch the other process requests to the device, which
> means that we have cases where we need to add printks to the kernel
> to understand what is happening.

Tracing is also nice, I like this too.

Anyway, I like the idea, I'm just worried we are pushing complexity out
into userspace which would make it "someone else's problem." The job of
a kernel is to provide a way to abstract devices in a standard way. To
force userspace to write a "new program per input device" would be a
total mess and a step backwards.

thanks,

greg k-h

2022-02-24 17:30:53

by Yonghong Song

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On 2/24/22 5:49 AM, Benjamin Tissoires wrote:
> Hi Greg,
>
> Thanks for the quick answer :)
>
> On Thu, Feb 24, 2022 at 12:31 PM Greg KH <[email protected]> wrote:
>>
>> On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
>>> Hi there,
>>>
>>> This series introduces support of eBPF for HID devices.
>>>
>>> I have several use cases where eBPF could be interesting for those
>>> input devices:
>>>
>>> - simple fixup of report descriptor:
>>>
>>> In the HID tree, we have half of the drivers that are "simple" and
>>> that just fix one key or one byte in the report descriptor.
>>> Currently, for users of such devices, the process of fixing them
>>> is long and painful.
>>> With eBPF, we could externalize those fixups in one external repo,
>>> ship various CoRe bpf programs and have those programs loaded at boot
>>> time without having to install a new kernel (and wait 6 months for the
>>> fix to land in the distro kernel)
>>
>> Why would a distro update such an external repo faster than they update
>> the kernel? Many sane distros update their kernel faster than other
>> packages already, how about fixing your distro? :)
>
> Heh, I'm going to try to dodge the incoming rhel bullet :)
>
> It's true that thanks to the work of the stable folks we don't have to
> wait 6 months for a fix to come in. However, I think having a single
> file to drop in a directory would be easier for development/testing
> (and distribution of that file between developers/testers) than
> requiring people to recompile their kernel.
>
> Brain fart: is there any chance we could keep the validated bpf
> programs in the kernel tree?

Yes, see kernel/bpf/preload/iterators/iterators.bpf.c.

>
>>
>> I'm all for the idea of using ebpf for HID devices, but now we have to
>> keep track of multiple packages to be in sync here. Is this making
>> things harder overall?
>
> Probably, and this is also maybe opening a can of worms. Vendors will
> be able to say "use that bpf program for my HID device because the
> firmware is bogus".
>
> OTOH, as far as I understand, you can not load a BPF program in the
> kernel that uses GPL-declared functions if your BPF program is not
> GPL. Which means that if firmware vendors want to distribute blobs
> through BPF, either it's GPL and they have to provide the sources, or
> it's not happening.
>
> I am not entirely clear on which plan I want to have for userspace.
> I'd like to have libinput on board, but right now, Peter's stance is
> "not in my garden" (and he has good reasons for it).
> So my initial plan is to cook and hold the bpf programs in hid-tools,
> which is the repo I am using for the regression tests on HID.
>
> I plan on building a systemd intrinsic that would detect the HID
> VID/PID and then load the various BPF programs associated with the
> small fixes.
> Note that everything can not be fixed through eBPF, simply because at
> boot we don't always have the root partition mounted.
[...]

2022-02-24 18:21:03

by Bastien Nocera

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Thu, 2022-02-24 at 12:31 +0100, Greg KH wrote:
> On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> > Hi there,
> >
> > This series introduces support of eBPF for HID devices.
> >
> > I have several use cases where eBPF could be interesting for those
> > input devices:
> >
> > - simple fixup of report descriptor:
> >
> > In the HID tree, we have half of the drivers that are "simple" and
> > that just fix one key or one byte in the report descriptor.
> > Currently, for users of such devices, the process of fixing them
> > is long and painful.
> > With eBPF, we could externalize those fixups in one external repo,
> > ship various CoRe bpf programs and have those programs loaded at
> > boot
> > time without having to install a new kernel (and wait 6 months for
> > the
> > fix to land in the distro kernel)
>
> Why would a distro update such an external repo faster than they
> update
> the kernel?  Many sane distros update their kernel faster than other
> packages already, how about fixing your distro?  :)
>
> I'm all for the idea of using ebpf for HID devices, but now we have
> to
> keep track of multiple packages to be in sync here.  Is this making
> things harder overall?

I don't quite understand how taking eBPF quirks for HID devices out of
the kernel tree is different from taking suspend quirks out of the
kernel tree:
https://www.spinics.net/lists/linux-usb/msg204506.html

2022-02-24 19:10:33

by Greg Kroah-Hartman

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Thu, Feb 24, 2022 at 06:41:18PM +0100, Bastien Nocera wrote:
> On Thu, 2022-02-24 at 12:31 +0100, Greg KH wrote:
> > On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> > > Hi there,
> > >
> > > This series introduces support of eBPF for HID devices.
> > >
> > > I have several use cases where eBPF could be interesting for those
> > > input devices:
> > >
> > > - simple fixup of report descriptor:
> > >
> > > In the HID tree, we have half of the drivers that are "simple" and
> > > that just fix one key or one byte in the report descriptor.
> > > Currently, for users of such devices, the process of fixing them
> > > is long and painful.
> > > With eBPF, we could externalize those fixups in one external repo,
> > > ship various CoRe bpf programs and have those programs loaded at
> > > boot
> > > time without having to install a new kernel (and wait 6 months for
> > > the
> > > fix to land in the distro kernel)
> >
> > Why would a distro update such an external repo faster than they
> > update
> > the kernel?? Many sane distros update their kernel faster than other
> > packages already, how about fixing your distro?? :)
> >
> > I'm all for the idea of using ebpf for HID devices, but now we have
> > to
> > keep track of multiple packages to be in sync here.? Is this making
> > things harder overall?
>
> I don't quite understand how taking eBPF quirks for HID devices out of
> the kernel tree is different from taking suspend quirks out of the
> kernel tree:
> https://www.spinics.net/lists/linux-usb/msg204506.html

A list of all devices possible, and the policy decisions to make on
those devices, belongs in userspace, not in the kernel. That's what the
hwdb contains.

Quirks in order to get the device to work properly is not a policy
decision, they are needed to get the device to work. If you wish to
suspend it or not based on the vendor/product id, in order to possibly
save some more battery life on some types of systems, is something that
belongs in userspace.

If you want to replace the existing HID quirk tables with an ebpf
program that ships with the kernel, wonderful, I have no objection to
that. If a user is required to download the external quirk table just
to get their device to work with the kernel, that's probably something
you don't want to do.

thanks,

greg k-h

2022-02-25 15:19:21

by Greg Kroah-Hartman

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Thu, Feb 24, 2022 at 02:49:21PM +0100, Benjamin Tissoires wrote:
> Hi Greg,
>
> Thanks for the quick answer :)
>
> On Thu, Feb 24, 2022 at 12:31 PM Greg KH <[email protected]> wrote:
> >
> > On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> > > Hi there,
> > >
> > > This series introduces support of eBPF for HID devices.
> > >
> > > I have several use cases where eBPF could be interesting for those
> > > input devices:
> > >
> > > - simple fixup of report descriptor:
> > >
> > > In the HID tree, we have half of the drivers that are "simple" and
> > > that just fix one key or one byte in the report descriptor.
> > > Currently, for users of such devices, the process of fixing them
> > > is long and painful.
> > > With eBPF, we could externalize those fixups in one external repo,
> > > ship various CoRe bpf programs and have those programs loaded at boot
> > > time without having to install a new kernel (and wait 6 months for the
> > > fix to land in the distro kernel)
> >
> > Why would a distro update such an external repo faster than they update
> > the kernel? Many sane distros update their kernel faster than other
> > packages already, how about fixing your distro? :)
>
> Heh, I'm going to try to dodge the incoming rhel bullet :)
>
> It's true that thanks to the work of the stable folks we don't have to
> wait 6 months for a fix to come in. However, I think having a single
> file to drop in a directory would be easier for development/testing
> (and distribution of that file between developers/testers) than
> requiring people to recompile their kernel.
>
> Brain fart: is there any chance we could keep the validated bpf
> programs in the kernel tree?

That would make the most sense to me. And allow "slow" distros to
override the HID bpf quirks easily if they need to. If you do that,
then most of my objections of the "now the code is in two places that
you have to track" goes away :)

> > I'm all for the idea of using ebpf for HID devices, but now we have to
> > keep track of multiple packages to be in sync here. Is this making
> > things harder overall?
>
> Probably, and this is also maybe opening a can of worms. Vendors will
> be able to say "use that bpf program for my HID device because the
> firmware is bogus".
>
> OTOH, as far as I understand, you can not load a BPF program in the
> kernel that uses GPL-declared functions if your BPF program is not
> GPL. Which means that if firmware vendors want to distribute blobs
> through BPF, either it's GPL and they have to provide the sources, or
> it's not happening.

You can make the new HID bpf api only availble to GPL programs, and if I
were you, that's what I would do just to keep any legal issues from
coming up. Also bundling it with the kernel makes it easier.

> I am not entirely clear on which plan I want to have for userspace.
> I'd like to have libinput on board, but right now, Peter's stance is
> "not in my garden" (and he has good reasons for it).
> So my initial plan is to cook and hold the bpf programs in hid-tools,
> which is the repo I am using for the regression tests on HID.

Why isn't the hid regression tests in the kernel tree also? That would
allow all of the testers out there to test things much easier than
having to suck down another test repo (like Linaro and 0-day and
kernelci would be forced to do).

> I plan on building a systemd intrinsic that would detect the HID
> VID/PID and then load the various BPF programs associated with the
> small fixes.
> Note that everything can not be fixed through eBPF, simply because at
> boot we don't always have the root partition mounted.

Root partitions are now on HID devices? :)

> > > - Universal Stylus Interface (or any other new fancy feature that
> > > requires a new kernel API)
> > >
> > > See [0].
> > > Basically, USI pens are requiring a new kernel API because there are
> > > some channels of communication our HID and input stack are not capable
> > > of. Instead of using hidraw or creating new sysfs or ioctls, we can rely
> > > on eBPF to have the kernel API controlled by the consumer and to not
> > > impact the performances by waking up userspace every time there is an
> > > event.
> >
> > How is userspace supposed to interact with these devices in a unified
> > way then? This would allow random new interfaces to be created, one
> > each for each device, and again, be a pain to track for a distro to keep
> > in sync. And how are you going to keep the ebpf interface these
> > provides in sync with the userspace program?
>
> Right now, the idea we have is to export the USI specifics through
> dbus. This has a couple of advantages: we are not tied to USI and can
> "emulate" those parameters by storing them on disk instead of in the
> pen, and this is easily accessible from all applications directly.
>
> I am trying to push to have one implementation of that dbus service
> with the Intel and ChromeOS folks so general linux doesn't have to
> recreate it. But if you look at it, with hidraw nothing prevents
> someone from writing such a library/daemon in its own world without
> sharing it with anybody.
>
> The main advantage of eBPF compared to hidraw is that you can analyse
> the incoming event without waking userspace and only wake it up when
> there is something noticeable.

That is a very good benefit, and one that many battery-powered devices
would like.

> In terms of random interfaces, yes, this is a good point. But the way
> I see it is that we can provide one kernel API (eBPF for HID) which we
> will maintain and not have to maintain forever a badly designed kernel
> API for a specific device. Though also note that USI is a HID standard
> (I think there is a second one), so AFAICT, the same bpf program
> should be able to be generic enough to be cross vendor. So there will
> be one provider only for USI.

Ok, that's good to know.

<good stuff snipped>

> Yeah, I completely understand the view. However, please keep in mind
> that most of it (though not firewall and some corner cases of tracing)
> is already possible to do through hidraw.
> One other example of that is SDL. We got Sony involved to create a
> nice driver for the DualSense controller (the PS5 one), but they
> simply ignore it and use plain HID (through hidraw). They have the
> advantage of this being cross-platform and can provide a consistent
> experience across platforms. And as a result, in the kernel, we have
> to hands up the handling of the device whenever somebody opens a
> hidraw node for those devices (Steam is also doing the same FWIW).
>
> Which reminds me that I also have another use-case: joystick
> dead-zone. You can have a small filter that configures the dead zone
> and doesn't even wake up userspace for those hardware glitches...

hidraw is a big issue, and I understand why vendors use that and prefer
it over writing a kernel driver. They can control it and ship it to
users and it makes life easier for them. It's also what Windows has
been doing for decades now, so it's a comfortable interface for them to
write their code in userspace.

But, now you are going to ask them to use bpf instead? Why would they
switch off of hidraw to use this api? What benefit are you going to
provide them here for that?

This is why I've delayed doing bpf for USB. Right now we have a nice
cross-platform way to write userspace USB drivers using usbfs/libusb.
All a bpf api to USB would be doing is much the same thing that libusb
does, for almost no real added benefit that I can tell.

USB, is a really "simple" networking like protocol (send/recieve
packets). So it ties into the bpf model well. But the need to use bpf
for USB so far I don't have a real justification.

And the same thing here. Yes it is cool, and personally I love it, but
what is going to get people off of hidraw to use this instead? You
can'd drop hidraw now (just like I can't drop usbfs), so all this means
is we have yet-another-way to do something on the system. Is that a
good idea? I don't know.

Also you mention dbus as the carrier for the HID information to
userspace programs. Is dbus really the right thing for sending streams
of input data around? Yes it will probably work, but I don't think it
was designed for that at all, so the overhead involved might just
overshadow any of the improvements you made using bpf. And also, you
could do this today with hidraw, right?

> Anyway, IOW, I think the bpf approach will allow kernel-like
> performances of hidraw applications, and I would be more inclined to
> ask people to move their weird issue in userspace thanks to that.

I like this from a "everyone should use bpf" point of view, but how are
you going to tell people "use bpf over hidraw" in a way that gets them
to do so? If you have a good answer for that, I might just steal it for
the bpf-USB interface as well :)

thanks,

greg k-h

2022-02-25 16:17:53

by Benjamin Tissoires

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Fri, Feb 25, 2022 at 2:38 PM Greg KH <[email protected]> wrote:
>
> On Thu, Feb 24, 2022 at 02:49:21PM +0100, Benjamin Tissoires wrote:
> > Hi Greg,
> >
> > Thanks for the quick answer :)
> >
> > On Thu, Feb 24, 2022 at 12:31 PM Greg KH <[email protected]> wrote:
> > >
> > > On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> > > > Hi there,
> > > >
> > > > This series introduces support of eBPF for HID devices.
> > > >
> > > > I have several use cases where eBPF could be interesting for those
> > > > input devices:
> > > >
> > > > - simple fixup of report descriptor:
> > > >
> > > > In the HID tree, we have half of the drivers that are "simple" and
> > > > that just fix one key or one byte in the report descriptor.
> > > > Currently, for users of such devices, the process of fixing them
> > > > is long and painful.
> > > > With eBPF, we could externalize those fixups in one external repo,
> > > > ship various CoRe bpf programs and have those programs loaded at boot
> > > > time without having to install a new kernel (and wait 6 months for the
> > > > fix to land in the distro kernel)
> > >
> > > Why would a distro update such an external repo faster than they update
> > > the kernel? Many sane distros update their kernel faster than other
> > > packages already, how about fixing your distro? :)
> >
> > Heh, I'm going to try to dodge the incoming rhel bullet :)
> >
> > It's true that thanks to the work of the stable folks we don't have to
> > wait 6 months for a fix to come in. However, I think having a single
> > file to drop in a directory would be easier for development/testing
> > (and distribution of that file between developers/testers) than
> > requiring people to recompile their kernel.
> >
> > Brain fart: is there any chance we could keep the validated bpf
> > programs in the kernel tree?
>
> That would make the most sense to me. And allow "slow" distros to
> override the HID bpf quirks easily if they need to. If you do that,
> then most of my objections of the "now the code is in two places that
> you have to track" goes away :)

Unfortunately, I don't think we will be able to have all the bpf
programs in the kernel.

I would say programs that "enable" a device (report descriptor fixup,
events override, single key remapping) which are there to make the
device fully functional could likely be stored and loaded by the
kernel. I have yet to figure out if the suggestion from Yonghong Song
allows me to also load the bpf program or if it's just a repo in the
tree for the sources.

However, for USI pens (or any other functionality that needs a new
high level kernel interface) I don't think we will have them here.
Unless we want to also bind the public API to dbus (or graybus maybe),
the bpf program should live in the userspace program so it can update
it and not be tied to the decisions in the kernel (I'll go more into
detail later in this reply).

So that's going to be a half and half solution :/

>
> > > I'm all for the idea of using ebpf for HID devices, but now we have to
> > > keep track of multiple packages to be in sync here. Is this making
> > > things harder overall?
> >
> > Probably, and this is also maybe opening a can of worms. Vendors will
> > be able to say "use that bpf program for my HID device because the
> > firmware is bogus".
> >
> > OTOH, as far as I understand, you can not load a BPF program in the
> > kernel that uses GPL-declared functions if your BPF program is not
> > GPL. Which means that if firmware vendors want to distribute blobs
> > through BPF, either it's GPL and they have to provide the sources, or
> > it's not happening.
>
> You can make the new HID bpf api only availble to GPL programs, and if I
> were you, that's what I would do just to keep any legal issues from
> coming up. Also bundling it with the kernel makes it easier.

Looking at kernel/bpf/bpf_lsm.c I can confirm that it should be easy
to prevent a program from binding to HID if it's not GPL :)

>
> > I am not entirely clear on which plan I want to have for userspace.
> > I'd like to have libinput on board, but right now, Peter's stance is
> > "not in my garden" (and he has good reasons for it).
> > So my initial plan is to cook and hold the bpf programs in hid-tools,
> > which is the repo I am using for the regression tests on HID.
>
> Why isn't the hid regression tests in the kernel tree also? That would
> allow all of the testers out there to test things much easier than
> having to suck down another test repo (like Linaro and 0-day and
> kernelci would be forced to do).

2 years ago I would have argued that the ease of development of
gitlab.fd.o was more suited to a fast moving project.

Now... The changes in the core part of the code don't change much so
yes, merging it in the kernel might have a lot of benefits outside of
what you said. The most immediate one is that I could require fixes to
be provided with a test, and merge them together, without having to
hold them until Linus releases a new version.

If nobody complains of having the regression tests in python with
pytest and some Python 3.6+ features, that is definitely something I
should look for.

>
> > I plan on building a systemd intrinsic that would detect the HID
> > VID/PID and then load the various BPF programs associated with the
> > small fixes.
> > Note that everything can not be fixed through eBPF, simply because at
> > boot we don't always have the root partition mounted.
>
> Root partitions are now on HID devices? :)

Sorry for not being clear :)

I mean that if you need a bpf program to be loaded from userspace at
boot to make your keyboard functional, then you need to have the root
partition mounted (or put the program in the initrd) so udev can load
it. Now if your keyboard is supposed to give the password used to
decrypt your root partition but you need a bpf program on that said
partition to make it functional, you are screwed :)

>
> > > > - Universal Stylus Interface (or any other new fancy feature that
> > > > requires a new kernel API)
> > > >
> > > > See [0].
> > > > Basically, USI pens are requiring a new kernel API because there are
> > > > some channels of communication our HID and input stack are not capable
> > > > of. Instead of using hidraw or creating new sysfs or ioctls, we can rely
> > > > on eBPF to have the kernel API controlled by the consumer and to not
> > > > impact the performances by waking up userspace every time there is an
> > > > event.
> > >
> > > How is userspace supposed to interact with these devices in a unified
> > > way then? This would allow random new interfaces to be created, one
> > > each for each device, and again, be a pain to track for a distro to keep
> > > in sync. And how are you going to keep the ebpf interface these
> > > provides in sync with the userspace program?
> >
> > Right now, the idea we have is to export the USI specifics through
> > dbus. This has a couple of advantages: we are not tied to USI and can
> > "emulate" those parameters by storing them on disk instead of in the
> > pen, and this is easily accessible from all applications directly.
> >
> > I am trying to push to have one implementation of that dbus service
> > with the Intel and ChromeOS folks so general linux doesn't have to
> > recreate it. But if you look at it, with hidraw nothing prevents
> > someone from writing such a library/daemon in its own world without
> > sharing it with anybody.
> >
> > The main advantage of eBPF compared to hidraw is that you can analyse
> > the incoming event without waking userspace and only wake it up when
> > there is something noticeable.
>
> That is a very good benefit, and one that many battery-powered devices
> would like.
>
> > In terms of random interfaces, yes, this is a good point. But the way
> > I see it is that we can provide one kernel API (eBPF for HID) which we
> > will maintain and not have to maintain forever a badly designed kernel
> > API for a specific device. Though also note that USI is a HID standard
> > (I think there is a second one), so AFAICT, the same bpf program
> > should be able to be generic enough to be cross vendor. So there will
> > be one provider only for USI.
>
> Ok, that's good to know.
>
> <good stuff snipped>
>
> > Yeah, I completely understand the view. However, please keep in mind
> > that most of it (though not firewall and some corner cases of tracing)
> > is already possible to do through hidraw.
> > One other example of that is SDL. We got Sony involved to create a
> > nice driver for the DualSense controller (the PS5 one), but they
> > simply ignore it and use plain HID (through hidraw). They have the
> > advantage of this being cross-platform and can provide a consistent
> > experience across platforms. And as a result, in the kernel, we have
> > to hands up the handling of the device whenever somebody opens a
> > hidraw node for those devices (Steam is also doing the same FWIW).
> >
> > Which reminds me that I also have another use-case: joystick
> > dead-zone. You can have a small filter that configures the dead zone
> > and doesn't even wake up userspace for those hardware glitches...
>
> hidraw is a big issue, and I understand why vendors use that and prefer
> it over writing a kernel driver. They can control it and ship it to
> users and it makes life easier for them. It's also what Windows has
> been doing for decades now, so it's a comfortable interface for them to
> write their code in userspace.
>
> But, now you are going to ask them to use bpf instead? Why would they
> switch off of hidraw to use this api? What benefit are you going to
> provide them here for that?

Again, there are 2 big classes of users of hid-bpf ("you" here is a
developer in general):
1. you need to fix your device
2. you need to add a new kernel API

2. can be entirely done with hidraw:
- you open the hidraw node
- you parse every incoming event
- out of that you build up your high level API

This is what we are doing in libratbag for instance to support gaming
mice that need extra features. We try to not read every single event,
but some mice are done in a way we don't have a choice.

With bpf, you could:
- load the bpf program
- have the kernel (bpf program) parse the incoming report without
waking up user space
- when something changes (a given button is pressed) the bpf program
notifies userspace with an event
- then userspace builds its own API on top of it (forward that change
through dbus for example)

As far as 1., the aim is not to replace hidraw but the kernel drivers
themselves:
instead of having a formal driver loaded in the kernel, you can rely
on a bpf program to do whatever needs to be done to make the device
working.

If the FW is wrong and the report descriptor messes up a button
mapping, you can change that with bpf instead of having a specific
driver for it.
And of course, using hidraw for that just doesn't work because the
event stream you get from hidraw is for the process only. In BPF, we
can change the event flow for anybody, which allows much more power
(but this is scarier too).

This class of bpf program should actually reside in the kernel tree so
everybody can benefit from it (circling back to your first point).

So I am not deprecating hidraw nor I am not asking them to use bpf instead.
But when you are interested in just one byte in the report, bpf allows
you to speed up your program and save battery.

>
> This is why I've delayed doing bpf for USB. Right now we have a nice
> cross-platform way to write userspace USB drivers using usbfs/libusb.
> All a bpf api to USB would be doing is much the same thing that libusb
> does, for almost no real added benefit that I can tell.
>
> USB, is a really "simple" networking like protocol (send/recieve
> packets). So it ties into the bpf model well. But the need to use bpf
> for USB so far I don't have a real justification.
>
> And the same thing here. Yes it is cool, and personally I love it, but
> what is going to get people off of hidraw to use this instead? You
> can'd drop hidraw now (just like I can't drop usbfs), so all this means
> is we have yet-another-way to do something on the system. Is that a
> good idea? I don't know.
>
> Also you mention dbus as the carrier for the HID information to
> userspace programs. Is dbus really the right thing for sending streams
> of input data around? Yes it will probably work, but I don't think it
> was designed for that at all, so the overhead involved might just
> overshadow any of the improvements you made using bpf. And also, you
> could do this today with hidraw, right?

Let me go into more details regarding USI.
Peter drafted a WIP for the dbus API at
https://gitlab.freedesktop.org/whot/usid. Unfortunately that's the
only tangent thing I can share right now for it, and it doesn't even
have the bpf part ;).

Basically the dbus API will export just the USI bits we care about:
- Preferred Color: the 8-bit Web color or 24 bit RGB color assigned to
the device
- Preferred Line Width: the physical width (e.g. 2mm)
- Preferred Line Style: One of Ink, Pencil, Highlighter, Chisel
Marker, Brush, No Preference.

The rest of the event stream will still continue to go through evdev,
and not dbus.

The bpf program we should have here parses the incoming report and
whenever there is a change in those 3 properties above, it raises an
event to the dbus daemon which will in turn forward that to its
clients.
On the other side, when a client needs to have a color change for
instance, it sets the dbus property and then the dbus daemon might
either store the data on disk or on the pen through bpf and HID.

So yes, you can do that with hidraw but that means you need to parse
all incoming reports in userspace for events that will likely occur
every once in a while.

In my mind, the dbus operations here are to replace the kernel API you
might want to create instead. In that case, Tero started with 2
in-kernel possibilities: a sysfs entry we could read/write, or a new
ioctl attached to the evdev node (it was actually a separate char
device).

>
> > Anyway, IOW, I think the bpf approach will allow kernel-like
> > performances of hidraw applications, and I would be more inclined to
> > ask people to move their weird issue in userspace thanks to that.
>
> I like this from a "everyone should use bpf" point of view, but how are
> you going to tell people "use bpf over hidraw" in a way that gets them
> to do so? If you have a good answer for that, I might just steal it for
> the bpf-USB interface as well :)
>

I don't think we can have a generic answer here unfortunately. It will
depend on the use case:
- SDL/Steam -> they are interested in the entire stream of events, so
we can't really tell them to use bpf, unless if they want to fix a
deadzone of a joystick
- if the idea is to fix the device (spurious event, wrong key mapping,
drift of one coordinate), hidraw doesn't apply, and bpf is IMO better
than writing a kernel module as long as we can ship them with the
kernel (because of the cost of compiling a driver/kernel and testing
is much higher than just inserting a BPF program in the running
kernel)
- if the idea is to add a new kernel API or functionality to a device
(a new haptic FF implementation, some USI properties, something unique
enough to not be generic and have a properly defined API), then bpf is
a good choice because by tying the bpf program with the high level API
from userspace we can ensure we don't have to maintain this kernel API
with all its bugs forever. And here, you could use hidraw but if you
need to filter a small amount of information in the event stream
instead of just using Set/Get reports, then BPF is much more
efficient.

I don't know enough about the USB use case you have for libusb (I know
mostly the input ones ;-P) to be able to give you an answer there.

Cheers,
Benjamin

2022-02-25 17:00:27

by Greg Kroah-Hartman

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Fri, Feb 25, 2022 at 05:06:32PM +0100, Benjamin Tissoires wrote:
> On Thu, Feb 24, 2022 at 6:21 PM Yonghong Song <[email protected]> wrote:
> >
> >
> >
> > On 2/24/22 5:49 AM, Benjamin Tissoires wrote:
> > > Hi Greg,
> > >
> > > Thanks for the quick answer :)
> > >
> > > On Thu, Feb 24, 2022 at 12:31 PM Greg KH <[email protected]> wrote:
> > >>
> > >> On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> > >>> Hi there,
> > >>>
> > >>> This series introduces support of eBPF for HID devices.
> > >>>
> > >>> I have several use cases where eBPF could be interesting for those
> > >>> input devices:
> > >>>
> > >>> - simple fixup of report descriptor:
> > >>>
> > >>> In the HID tree, we have half of the drivers that are "simple" and
> > >>> that just fix one key or one byte in the report descriptor.
> > >>> Currently, for users of such devices, the process of fixing them
> > >>> is long and painful.
> > >>> With eBPF, we could externalize those fixups in one external repo,
> > >>> ship various CoRe bpf programs and have those programs loaded at boot
> > >>> time without having to install a new kernel (and wait 6 months for the
> > >>> fix to land in the distro kernel)
> > >>
> > >> Why would a distro update such an external repo faster than they update
> > >> the kernel? Many sane distros update their kernel faster than other
> > >> packages already, how about fixing your distro? :)
> > >
> > > Heh, I'm going to try to dodge the incoming rhel bullet :)
> > >
> > > It's true that thanks to the work of the stable folks we don't have to
> > > wait 6 months for a fix to come in. However, I think having a single
> > > file to drop in a directory would be easier for development/testing
> > > (and distribution of that file between developers/testers) than
> > > requiring people to recompile their kernel.
> > >
> > > Brain fart: is there any chance we could keep the validated bpf
> > > programs in the kernel tree?
> >
> > Yes, see kernel/bpf/preload/iterators/iterators.bpf.c.
>
> Thanks. This is indeed interesting.
> I am not sure the exact usage of it though :)
>
> One thing I wonder too while we are on this topic, is it possible to
> load a bpf program from the kernel directly, in the same way we can
> request firmwares?

We used to be able to do that, putting bpf programs inside a module.
But that might have gotten removed because no one actually used it. I
thought it was a nice idea.

> Because if we can do that, in my HID use case we could replace simple
> drivers with bpf programs entirely and reduce the development cycle to
> a bare minimum.

How would the development cycle change? You could get rid of many
in-kernel hid drivers and replace them with bpf code perhaps? Maybe
that's a good use case :)

thanks,

greg k-h

2022-02-25 17:34:10

by Yonghong Song

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On 2/25/22 8:06 AM, Benjamin Tissoires wrote:
> On Thu, Feb 24, 2022 at 6:21 PM Yonghong Song <[email protected]> wrote:
>>
>>
>>
>> On 2/24/22 5:49 AM, Benjamin Tissoires wrote:
>>> Hi Greg,
>>>
>>> Thanks for the quick answer :)
>>>
>>> On Thu, Feb 24, 2022 at 12:31 PM Greg KH <[email protected]> wrote:
>>>>
>>>> On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
>>>>> Hi there,
>>>>>
>>>>> This series introduces support of eBPF for HID devices.
>>>>>
>>>>> I have several use cases where eBPF could be interesting for those
>>>>> input devices:
>>>>>
>>>>> - simple fixup of report descriptor:
>>>>>
>>>>> In the HID tree, we have half of the drivers that are "simple" and
>>>>> that just fix one key or one byte in the report descriptor.
>>>>> Currently, for users of such devices, the process of fixing them
>>>>> is long and painful.
>>>>> With eBPF, we could externalize those fixups in one external repo,
>>>>> ship various CoRe bpf programs and have those programs loaded at boot
>>>>> time without having to install a new kernel (and wait 6 months for the
>>>>> fix to land in the distro kernel)
>>>>
>>>> Why would a distro update such an external repo faster than they update
>>>> the kernel? Many sane distros update their kernel faster than other
>>>> packages already, how about fixing your distro? :)
>>>
>>> Heh, I'm going to try to dodge the incoming rhel bullet :)
>>>
>>> It's true that thanks to the work of the stable folks we don't have to
>>> wait 6 months for a fix to come in. However, I think having a single
>>> file to drop in a directory would be easier for development/testing
>>> (and distribution of that file between developers/testers) than
>>> requiring people to recompile their kernel.
>>>
>>> Brain fart: is there any chance we could keep the validated bpf
>>> programs in the kernel tree?
>>
>> Yes, see kernel/bpf/preload/iterators/iterators.bpf.c.
>
> Thanks. This is indeed interesting.
> I am not sure the exact usage of it though :)
>
> One thing I wonder too while we are on this topic, is it possible to
> load a bpf program from the kernel directly, in the same way we can
> request firmwares?

Yes. You can. See the example at kernel/bpf/preload directory.
The example will pin the link (holding a reference to the program)
into bpffs (implemented in kernel/bpf/inode.c).

Later on, in userspace, you can grab a fd from bpffs pinned link and use
BPF_LINK_UPDATE to update the program if you want. This way,
if your driver uses the link to get the program, they will
automatically get the new program in the next run.

>
> Because if we can do that, in my HID use case we could replace simple
> drivers with bpf programs entirely and reduce the development cycle to
> a bare minimum. >
> Cheers,
> Benjamin
>
>
>>
>>>
>>>>
>>>> I'm all for the idea of using ebpf for HID devices, but now we have to
>>>> keep track of multiple packages to be in sync here. Is this making
>>>> things harder overall?
>>>
>>> Probably, and this is also maybe opening a can of worms. Vendors will
>>> be able to say "use that bpf program for my HID device because the
>>> firmware is bogus".
>>>
>>> OTOH, as far as I understand, you can not load a BPF program in the
>>> kernel that uses GPL-declared functions if your BPF program is not
>>> GPL. Which means that if firmware vendors want to distribute blobs
>>> through BPF, either it's GPL and they have to provide the sources, or
>>> it's not happening.
>>>
>>> I am not entirely clear on which plan I want to have for userspace.
>>> I'd like to have libinput on board, but right now, Peter's stance is
>>> "not in my garden" (and he has good reasons for it).
>>> So my initial plan is to cook and hold the bpf programs in hid-tools,
>>> which is the repo I am using for the regression tests on HID.
>>>
>>> I plan on building a systemd intrinsic that would detect the HID
>>> VID/PID and then load the various BPF programs associated with the
>>> small fixes.
>>> Note that everything can not be fixed through eBPF, simply because at
>>> boot we don't always have the root partition mounted.
>> [...]
>>
>

2022-02-25 18:02:08

by Greg Kroah-Hartman

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Fri, Feb 25, 2022 at 05:00:53PM +0100, Benjamin Tissoires wrote:
> > > I plan on building a systemd intrinsic that would detect the HID
> > > VID/PID and then load the various BPF programs associated with the
> > > small fixes.
> > > Note that everything can not be fixed through eBPF, simply because at
> > > boot we don't always have the root partition mounted.
> >
> > Root partitions are now on HID devices? :)
>
> Sorry for not being clear :)
>
> I mean that if you need a bpf program to be loaded from userspace at
> boot to make your keyboard functional, then you need to have the root
> partition mounted (or put the program in the initrd) so udev can load
> it. Now if your keyboard is supposed to give the password used to
> decrypt your root partition but you need a bpf program on that said
> partition to make it functional, you are screwed :)

True, but that's why the HID boot protocol was designed for keyboards
and mice, so that they "always" work. Yeah, I know many devices ignore
it, oh well...

Anyway, the requirement of "if you need it to boot, don't make it a bpf
program" is fine, unless you can put the bpf program in a kernel module
(see my other response for that.)

> > > > > - Universal Stylus Interface (or any other new fancy feature that
> > > > > requires a new kernel API)
> > > > >
> > > > > See [0].
> > > > > Basically, USI pens are requiring a new kernel API because there are
> > > > > some channels of communication our HID and input stack are not capable
> > > > > of. Instead of using hidraw or creating new sysfs or ioctls, we can rely
> > > > > on eBPF to have the kernel API controlled by the consumer and to not
> > > > > impact the performances by waking up userspace every time there is an
> > > > > event.
> > > >
> > > > How is userspace supposed to interact with these devices in a unified
> > > > way then? This would allow random new interfaces to be created, one
> > > > each for each device, and again, be a pain to track for a distro to keep
> > > > in sync. And how are you going to keep the ebpf interface these
> > > > provides in sync with the userspace program?
> > >
> > > Right now, the idea we have is to export the USI specifics through
> > > dbus. This has a couple of advantages: we are not tied to USI and can
> > > "emulate" those parameters by storing them on disk instead of in the
> > > pen, and this is easily accessible from all applications directly.
> > >
> > > I am trying to push to have one implementation of that dbus service
> > > with the Intel and ChromeOS folks so general linux doesn't have to
> > > recreate it. But if you look at it, with hidraw nothing prevents
> > > someone from writing such a library/daemon in its own world without
> > > sharing it with anybody.
> > >
> > > The main advantage of eBPF compared to hidraw is that you can analyse
> > > the incoming event without waking userspace and only wake it up when
> > > there is something noticeable.
> >
> > That is a very good benefit, and one that many battery-powered devices
> > would like.
> >
> > > In terms of random interfaces, yes, this is a good point. But the way
> > > I see it is that we can provide one kernel API (eBPF for HID) which we
> > > will maintain and not have to maintain forever a badly designed kernel
> > > API for a specific device. Though also note that USI is a HID standard
> > > (I think there is a second one), so AFAICT, the same bpf program
> > > should be able to be generic enough to be cross vendor. So there will
> > > be one provider only for USI.
> >
> > Ok, that's good to know.
> >
> > <good stuff snipped>
> >
> > > Yeah, I completely understand the view. However, please keep in mind
> > > that most of it (though not firewall and some corner cases of tracing)
> > > is already possible to do through hidraw.
> > > One other example of that is SDL. We got Sony involved to create a
> > > nice driver for the DualSense controller (the PS5 one), but they
> > > simply ignore it and use plain HID (through hidraw). They have the
> > > advantage of this being cross-platform and can provide a consistent
> > > experience across platforms. And as a result, in the kernel, we have
> > > to hands up the handling of the device whenever somebody opens a
> > > hidraw node for those devices (Steam is also doing the same FWIW).
> > >
> > > Which reminds me that I also have another use-case: joystick
> > > dead-zone. You can have a small filter that configures the dead zone
> > > and doesn't even wake up userspace for those hardware glitches...
> >
> > hidraw is a big issue, and I understand why vendors use that and prefer
> > it over writing a kernel driver. They can control it and ship it to
> > users and it makes life easier for them. It's also what Windows has
> > been doing for decades now, so it's a comfortable interface for them to
> > write their code in userspace.
> >
> > But, now you are going to ask them to use bpf instead? Why would they
> > switch off of hidraw to use this api? What benefit are you going to
> > provide them here for that?
>
> Again, there are 2 big classes of users of hid-bpf ("you" here is a
> developer in general):
> 1. you need to fix your device
> 2. you need to add a new kernel API
>
> 2. can be entirely done with hidraw:
> - you open the hidraw node
> - you parse every incoming event
> - out of that you build up your high level API
>
> This is what we are doing in libratbag for instance to support gaming
> mice that need extra features. We try to not read every single event,
> but some mice are done in a way we don't have a choice.
>
> With bpf, you could:
> - load the bpf program
> - have the kernel (bpf program) parse the incoming report without
> waking up user space
> - when something changes (a given button is pressed) the bpf program
> notifies userspace with an event
> - then userspace builds its own API on top of it (forward that change
> through dbus for example)
>
> As far as 1., the aim is not to replace hidraw but the kernel drivers
> themselves:
> instead of having a formal driver loaded in the kernel, you can rely
> on a bpf program to do whatever needs to be done to make the device
> working.
>
> If the FW is wrong and the report descriptor messes up a button
> mapping, you can change that with bpf instead of having a specific
> driver for it.
> And of course, using hidraw for that just doesn't work because the
> event stream you get from hidraw is for the process only. In BPF, we
> can change the event flow for anybody, which allows much more power
> (but this is scarier too).
>
> This class of bpf program should actually reside in the kernel tree so
> everybody can benefit from it (circling back to your first point).
>
> So I am not deprecating hidraw nor I am not asking them to use bpf instead.
> But when you are interested in just one byte in the report, bpf allows
> you to speed up your program and save battery.

Ah, so really you are using bpf here as a "filter" for the HID events
that you care about to send to userspace or act apon in some way. That
makes a lot more sense to me, sorry for not realizing it sooner.

So yes, I agree, HID control with bpf does make sense. You can fix up
and filter out only the events you want without getting userspace
involved if it does not match. That should make people's lives easier
(hopefully) and based on your example code you provide in this patch
series, it doesn't look all that complex.

Along this line, now I think I know what we can do for USB with bpf as
well. Much the same thing, like a smart filter, which is what bpf was
designed for. USB is just a stream of data like a network connection
with pipes and the like, so it will work quite well for this.

Ok, thanks for the explanations, you've sold me, nice work :)

One comment about the patch series. You might want to break the patches
up a bit smaller, having the example code in a separate commit from the
"add this feature" commit, as it was hard to pick out what was kernel
changes, and what was test changes from it. That way I can complain
about the example code and tests without having to worry about the
kernel patches.

thanks,

greg k-h

2022-02-25 22:09:30

by Benjamin Tissoires

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Thu, Feb 24, 2022 at 6:21 PM Yonghong Song <[email protected]> wrote:
>
>
>
> On 2/24/22 5:49 AM, Benjamin Tissoires wrote:
> > Hi Greg,
> >
> > Thanks for the quick answer :)
> >
> > On Thu, Feb 24, 2022 at 12:31 PM Greg KH <[email protected]> wrote:
> >>
> >> On Thu, Feb 24, 2022 at 12:08:22PM +0100, Benjamin Tissoires wrote:
> >>> Hi there,
> >>>
> >>> This series introduces support of eBPF for HID devices.
> >>>
> >>> I have several use cases where eBPF could be interesting for those
> >>> input devices:
> >>>
> >>> - simple fixup of report descriptor:
> >>>
> >>> In the HID tree, we have half of the drivers that are "simple" and
> >>> that just fix one key or one byte in the report descriptor.
> >>> Currently, for users of such devices, the process of fixing them
> >>> is long and painful.
> >>> With eBPF, we could externalize those fixups in one external repo,
> >>> ship various CoRe bpf programs and have those programs loaded at boot
> >>> time without having to install a new kernel (and wait 6 months for the
> >>> fix to land in the distro kernel)
> >>
> >> Why would a distro update such an external repo faster than they update
> >> the kernel? Many sane distros update their kernel faster than other
> >> packages already, how about fixing your distro? :)
> >
> > Heh, I'm going to try to dodge the incoming rhel bullet :)
> >
> > It's true that thanks to the work of the stable folks we don't have to
> > wait 6 months for a fix to come in. However, I think having a single
> > file to drop in a directory would be easier for development/testing
> > (and distribution of that file between developers/testers) than
> > requiring people to recompile their kernel.
> >
> > Brain fart: is there any chance we could keep the validated bpf
> > programs in the kernel tree?
>
> Yes, see kernel/bpf/preload/iterators/iterators.bpf.c.

Thanks. This is indeed interesting.
I am not sure the exact usage of it though :)

One thing I wonder too while we are on this topic, is it possible to
load a bpf program from the kernel directly, in the same way we can
request firmwares?

Because if we can do that, in my HID use case we could replace simple
drivers with bpf programs entirely and reduce the development cycle to
a bare minimum.

Cheers,
Benjamin


>
> >
> >>
> >> I'm all for the idea of using ebpf for HID devices, but now we have to
> >> keep track of multiple packages to be in sync here. Is this making
> >> things harder overall?
> >
> > Probably, and this is also maybe opening a can of worms. Vendors will
> > be able to say "use that bpf program for my HID device because the
> > firmware is bogus".
> >
> > OTOH, as far as I understand, you can not load a BPF program in the
> > kernel that uses GPL-declared functions if your BPF program is not
> > GPL. Which means that if firmware vendors want to distribute blobs
> > through BPF, either it's GPL and they have to provide the sources, or
> > it's not happening.
> >
> > I am not entirely clear on which plan I want to have for userspace.
> > I'd like to have libinput on board, but right now, Peter's stance is
> > "not in my garden" (and he has good reasons for it).
> > So my initial plan is to cook and hold the bpf programs in hid-tools,
> > which is the repo I am using for the regression tests on HID.
> >
> > I plan on building a systemd intrinsic that would detect the HID
> > VID/PID and then load the various BPF programs associated with the
> > small fixes.
> > Note that everything can not be fixed through eBPF, simply because at
> > boot we don't always have the root partition mounted.
> [...]
>

2022-02-26 01:47:43

by Greg Kroah-Hartman

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

HID selftests question for now:

On Fri, Feb 25, 2022 at 05:00:53PM +0100, Benjamin Tissoires wrote:
> > > I am not entirely clear on which plan I want to have for userspace.
> > > I'd like to have libinput on board, but right now, Peter's stance is
> > > "not in my garden" (and he has good reasons for it).
> > > So my initial plan is to cook and hold the bpf programs in hid-tools,
> > > which is the repo I am using for the regression tests on HID.
> >
> > Why isn't the hid regression tests in the kernel tree also? That would
> > allow all of the testers out there to test things much easier than
> > having to suck down another test repo (like Linaro and 0-day and
> > kernelci would be forced to do).
>
> 2 years ago I would have argued that the ease of development of
> gitlab.fd.o was more suited to a fast moving project.
>
> Now... The changes in the core part of the code don't change much so
> yes, merging it in the kernel might have a lot of benefits outside of
> what you said. The most immediate one is that I could require fixes to
> be provided with a test, and merge them together, without having to
> hold them until Linus releases a new version.

Yes, having a test be required for a fix is a great idea. Many
subsystems do this already and it helps a lot.

> If nobody complains of having the regression tests in python with
> pytest and some Python 3.6+ features, that is definitely something I
> should look for.

Look at the tools/testing/selftests/ directory today. We already have
python3 tests in there, and as long as you follow the proper TAP output
format, all should be fine. The tc-testing python code in the kernel
trees seems to do that and no one has complained yet :)

thanks,

greg k-h

2022-02-26 07:38:08

by Song Liu

Subject: Re: [PATCH bpf-next v1 2/6] HID: bpf: allow to change the report descriptor from an eBPF program

On Thu, Feb 24, 2022 at 3:09 AM Benjamin Tissoires
<[email protected]> wrote:
>
> The report descriptor is the dictionary of the HID protocol specific
> to the given device.
> Changing it is a common habit in the HID world, and making that feature
> accessible from eBPF allows to fix devices without having to install a
> new kernel.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>

[...]

> 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
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 5978b92cacd3..a7a8d9cfcf24 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -999,6 +999,7 @@ enum bpf_attach_type {
> BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
> BPF_PERF_EVENT,
> BPF_HID_DEVICE_EVENT,
> + BPF_HID_RDESC_FIXUP,
> __MAX_BPF_ATTACH_TYPE
> };
>
> diff --git a/include/uapi/linux/bpf_hid.h b/include/uapi/linux/bpf_hid.h
> index 243ac45a253f..c0801d7174c3 100644
> --- a/include/uapi/linux/bpf_hid.h
> +++ b/include/uapi/linux/bpf_hid.h
> @@ -18,6 +18,7 @@ struct hid_device;
> enum hid_bpf_event {
> HID_BPF_UNDEF = 0,
> HID_BPF_DEVICE_EVENT,
> + HID_BPF_RDESC_FIXUP,
> };
>
> /* type is HID_BPF_DEVICE_EVENT */
> @@ -26,12 +27,19 @@ struct hid_bpf_ctx_device_event {
> unsigned long size;
> };
>
> +/* type is HID_BPF_RDESC_FIXUP */
> +struct hid_bpf_ctx_rdesc_fixup {
> + __u8 data[HID_BPF_MAX_BUFFER_SIZE];
> + unsigned long size;
> +};

This looks same as HID_BPF_DEVICE_EVENT, do we really need to
separate the two?

Thanks,
Song

2022-02-26 07:46:29

by Song Liu

Subject: Re: [PATCH bpf-next v1 1/6] HID: initial BPF implementation

On Thu, Feb 24, 2022 at 3:09 AM Benjamin Tissoires
<[email protected]> wrote:
>
> HID is a protocol that could benefit from using BPF too.
>
> This patch implements a net-like use of BPF capability for HID.
> Any incoming report coming from the device gets injected into a series
> of BPF programs that can modify it or even discard it by setting the
> size in the context to 0.
>
> The kernel/bpf implementation is based on net-namespace.c, with only
> the bpf_link part kept, there is no real points in keeping the
> bpf_prog_{attach|detach} API.
>
> The implementation is split into 2 parts:
> - the kernel/bpf part which isn't aware of the HID usage, but takes care
> of handling the BPF links
> - the drivers/hid/hid-bpf.c part which knows about HID
>
> Note that HID can be compiled in as a module, and so the functions that
> kernel/bpf/hid.c needs to call in hid.ko are exported in struct hid_hooks.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-bpf.c | 176 ++++++++
> drivers/hid/hid-core.c | 21 +-
> include/linux/bpf-hid.h | 87 ++++
> include/linux/bpf_types.h | 4 +
> include/linux/hid.h | 16 +
> include/uapi/linux/bpf.h | 7 +
> include/uapi/linux/bpf_hid.h | 39 ++
> kernel/bpf/Makefile | 3 +
> kernel/bpf/hid.c | 437 +++++++++++++++++++
> kernel/bpf/syscall.c | 8 +
> samples/bpf/.gitignore | 1 +
> samples/bpf/Makefile | 4 +
> samples/bpf/hid_mouse_kern.c | 66 +++
> samples/bpf/hid_mouse_user.c | 129 ++++++
> tools/include/uapi/linux/bpf.h | 7 +
> tools/lib/bpf/libbpf.c | 7 +
> tools/lib/bpf/libbpf.h | 2 +
> tools/lib/bpf/libbpf.map | 1 +
> tools/testing/selftests/bpf/prog_tests/hid.c | 318 ++++++++++++++
> tools/testing/selftests/bpf/progs/hid.c | 20 +
> 21 files changed, 1351 insertions(+), 3 deletions(-)
> 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

Please split kernel changes, libbpf changes, selftests, and sample code into
separate patches.

>
> 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..6c8445820944
> --- /dev/null
> +++ b/drivers/hid/hid-bpf.c
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * BPF in HID support for Linux
> + *
> + * Copyright (c) 2021 Benjamin Tissoires

Maybe 2022?

[...]

> +static int hid_bpf_run_progs(struct hid_device *hdev, enum bpf_hid_attach_type type,
> + struct hid_bpf_ctx *ctx, u8 *data, int size)
> +{
> + enum hid_bpf_event event = HID_BPF_UNDEF;
> +
> + if (type < 0 || !ctx)
> + return -EINVAL;
> +
> + switch (type) {
> + case BPF_HID_ATTACH_DEVICE_EVENT:
> + event = HID_BPF_DEVICE_EVENT;
> + if (size > sizeof(ctx->u.device.data))
> + return -E2BIG;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (!hdev->bpf.run_array[type])
> + return 0;
> +
> + memset(ctx, 0, sizeof(*ctx));
> + ctx->hdev = hdev;
> + ctx->type = event;
> +
> + if (size && data) {
> + switch (event) {
> + case HID_BPF_DEVICE_EVENT:
> + memcpy(ctx->u.device.data, data, size);
> + ctx->u.device.size = size;
> + break;
> + default:
> + /* do nothing */
> + }
> + }
> +
> + BPF_PROG_RUN_ARRAY(hdev->bpf.run_array[type], ctx, bpf_prog_run);

I guess we need "return BPF_PROG_RUN_ARRAY(...)"?

> +
> + return 0;
> +}
> +
> +u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *data, int *size)
> +{
> + int ret;
> +
> + if (bpf_hid_link_empty(&hdev->bpf, BPF_HID_ATTACH_DEVICE_EVENT))
> + return data;
> +
> + ret = hid_bpf_run_progs(hdev, BPF_HID_ATTACH_DEVICE_EVENT,
> + hdev->bpf.ctx, data, *size);
> + if (ret)
> + return data;

shall we return ERR_PTR(ret)?

> +
> + if (!hdev->bpf.ctx->u.device.size)
> + return ERR_PTR(-EINVAL);
> +
> + *size = hdev->bpf.ctx->u.device.size;
> +
> + return hdev->bpf.ctx->u.device.data;
> +}

[...]

> diff --git a/include/uapi/linux/bpf_hid.h b/include/uapi/linux/bpf_hid.h
> new file mode 100644
> index 000000000000..243ac45a253f
> --- /dev/null
> +++ b/include/uapi/linux/bpf_hid.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */
> +
> +/*
> + * HID BPF public headers
> + *
> + * Copyright (c) 2021 Benjamin Tissoires
> + */
> +
> +#ifndef _UAPI__LINUX_BPF_HID_H__
> +#define _UAPI__LINUX_BPF_HID_H__
> +
> +#include <linux/types.h>
> +
> +#define HID_BPF_MAX_BUFFER_SIZE 16384 /* 16kb */
> +
> +struct hid_device;
> +
> +enum hid_bpf_event {
> + HID_BPF_UNDEF = 0,
> + HID_BPF_DEVICE_EVENT,
> +};
> +
> +/* type is HID_BPF_DEVICE_EVENT */
> +struct hid_bpf_ctx_device_event {
> + __u8 data[HID_BPF_MAX_BUFFER_SIZE];

16kB sounds pretty big to me, do we usually need that much?

> + unsigned long size;
> +};
> +
> +struct hid_bpf_ctx {
> + enum hid_bpf_event type;
> + struct hid_device *hdev;
> +
> + union {
> + struct hid_bpf_ctx_device_event device;
> + } u;
> +};
> +
> +#endif /* _UAPI__LINUX_BPF_HID_H__ */
[...]

> diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c
> new file mode 100644
> index 000000000000..d3cb952bfc26
> --- /dev/null
> +++ b/kernel/bpf/hid.c

[...]

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 9c7a72b65eee..230ca6964a7e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3,6 +3,7 @@
> */
> #include <linux/bpf.h>
> #include <linux/bpf-cgroup.h>
> +#include <linux/bpf-hid.h>
> #include <linux/bpf_trace.h>
> #include <linux/bpf_lirc.h>
> #include <linux/bpf_verifier.h>
> @@ -2174,6 +2175,7 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type)
> case BPF_PROG_TYPE_CGROUP_SYSCTL:
> case BPF_PROG_TYPE_SOCK_OPS:
> case BPF_PROG_TYPE_EXT: /* extends any prog */
> + case BPF_PROG_TYPE_HID:

Is this net_admin type?

> return true;
> case BPF_PROG_TYPE_CGROUP_SKB:
> /* always unpriv */
> @@ -3188,6 +3190,8 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
> return BPF_PROG_TYPE_SK_LOOKUP;
> case BPF_XDP:
> return BPF_PROG_TYPE_XDP;
> + case BPF_HID_DEVICE_EVENT:
> + return BPF_PROG_TYPE_HID;
> default:
> return BPF_PROG_TYPE_UNSPEC;
> }
> @@ -3331,6 +3335,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
> case BPF_SK_MSG_VERDICT:
> case BPF_SK_SKB_VERDICT:
> return sock_map_bpf_prog_query(attr, uattr);
> + case BPF_HID_DEVICE_EVENT:
> + return bpf_hid_prog_query(attr, uattr);
> default:
> return -EINVAL;
> }
> @@ -4325,6 +4331,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> ret = bpf_perf_link_attach(attr, prog);
> break;
> #endif
> + case BPF_PROG_TYPE_HID:
> + return bpf_hid_link_create(attr, prog);
> default:
> ret = -EINVAL;
> }

[...]

> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index afe3d0d7f5f2..5978b92cacd3 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,7 @@ enum bpf_attach_type {
> BPF_SK_REUSEPORT_SELECT,
> BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
> BPF_PERF_EVENT,
> + BPF_HID_DEVICE_EVENT,
> __MAX_BPF_ATTACH_TYPE
> };
>
> @@ -1011,6 +1013,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,
> };
> @@ -5870,6 +5873,10 @@ struct bpf_link_info {
> struct {
> __u32 ifindex;
> } xdp;
> + struct {
> + __s32 hidraw_ino;
> + __u32 attach_type;
> + } hid;
> };
> } __attribute__((aligned(8)));
>

2022-02-26 07:46:51

by Song Liu

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Fri, Feb 25, 2022 at 8:32 AM Greg KH <[email protected]> wrote:
>
[...]
>
> One comment about the patch series. You might want to break the patches
> up a bit smaller, having the example code in a separate commit from the
> "add this feature" commit, as it was hard to pick out what was kernel
> changes, and what was test changes from it. That way I can complain
> about the example code and tests without having to worry about the
> kernel patches.

Echo on this part. Please organize kernel changes, libbpf changes,
maybe also bpftool changes, selftests, and samples into separate patches.
This would help folks without HID experience understand the design.

Thanks,
Song

2022-02-26 13:37:58

by Benjamin Tissoires

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Sat, Feb 26, 2022 at 8:37 AM Song Liu <[email protected]> wrote:
>
> On Fri, Feb 25, 2022 at 8:32 AM Greg KH <[email protected]> wrote:
> >
> [...]
> >
> > One comment about the patch series. You might want to break the patches
> > up a bit smaller, having the example code in a separate commit from the
> > "add this feature" commit, as it was hard to pick out what was kernel
> > changes, and what was test changes from it. That way I can complain
> > about the example code and tests without having to worry about the
> > kernel patches.
>
> Echo on this part. Please organize kernel changes, libbpf changes,
> maybe also bpftool changes, selftests, and samples into separate patches.
> This would help folks without HID experience understand the design.
>

Sure. And thanks for the initial review.

I'll send out a splitted v2 early next week then.

Cheers,
Benjamin

2022-02-28 18:05:27

by Jiri Kosina

Subject: Re: [PATCH bpf-next v1 0/6] Introduce eBPF support for HID devices

On Fri, 25 Feb 2022, Greg KH wrote:

> > I mean that if you need a bpf program to be loaded from userspace at
> > boot to make your keyboard functional, then you need to have the root
> > partition mounted (or put the program in the initrd) so udev can load
> > it. Now if your keyboard is supposed to give the password used to
> > decrypt your root partition but you need a bpf program on that said
> > partition to make it functional, you are screwed :)
>
> True, but that's why the HID boot protocol was designed for keyboards
> and mice, so that they "always" work. Yeah, I know many devices ignore
> it, oh well...

That's a very mild statement :)

*Most* of the recent modern HW doesn't support it as far as I can say.

Thanks,

--
Jiri Kosina
SUSE Labs

2022-02-28 19:01:28

by Benjamin Tissoires

Subject: Re: [PATCH bpf-next v1 1/6] HID: initial BPF implementation

On Sat, Feb 26, 2022 at 8:27 AM Song Liu <[email protected]> wrote:
>
> On Thu, Feb 24, 2022 at 3:09 AM Benjamin Tissoires
> <[email protected]> wrote:
> >
> > HID is a protocol that could benefit from using BPF too.
> >
> > This patch implements a net-like use of BPF capability for HID.
> > Any incoming report coming from the device gets injected into a series
> > of BPF programs that can modify it or even discard it by setting the
> > size in the context to 0.
> >
> > The kernel/bpf implementation is based on net-namespace.c, with only
> > the bpf_link part kept, there is no real points in keeping the
> > bpf_prog_{attach|detach} API.
> >
> > The implementation is split into 2 parts:
> > - the kernel/bpf part which isn't aware of the HID usage, but takes care
> > of handling the BPF links
> > - the drivers/hid/hid-bpf.c part which knows about HID
> >
> > Note that HID can be compiled in as a module, and so the functions that
> > kernel/bpf/hid.c needs to call in hid.ko are exported in struct hid_hooks.
> >
> > Signed-off-by: Benjamin Tissoires <[email protected]>
> > ---
> > drivers/hid/Makefile | 1 +
> > drivers/hid/hid-bpf.c | 176 ++++++++
> > drivers/hid/hid-core.c | 21 +-
> > include/linux/bpf-hid.h | 87 ++++
> > include/linux/bpf_types.h | 4 +
> > include/linux/hid.h | 16 +
> > include/uapi/linux/bpf.h | 7 +
> > include/uapi/linux/bpf_hid.h | 39 ++
> > kernel/bpf/Makefile | 3 +
> > kernel/bpf/hid.c | 437 +++++++++++++++++++
> > kernel/bpf/syscall.c | 8 +
> > samples/bpf/.gitignore | 1 +
> > samples/bpf/Makefile | 4 +
> > samples/bpf/hid_mouse_kern.c | 66 +++
> > samples/bpf/hid_mouse_user.c | 129 ++++++
> > tools/include/uapi/linux/bpf.h | 7 +
> > tools/lib/bpf/libbpf.c | 7 +
> > tools/lib/bpf/libbpf.h | 2 +
> > tools/lib/bpf/libbpf.map | 1 +
> > tools/testing/selftests/bpf/prog_tests/hid.c | 318 ++++++++++++++
> > tools/testing/selftests/bpf/progs/hid.c | 20 +
> > 21 files changed, 1351 insertions(+), 3 deletions(-)
> > 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
>
> Please split kernel changes, libbpf changes, selftests, and sample code into
> separate patches.

OK, done locally.

>
> >
> > 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..6c8445820944
> > --- /dev/null
> > +++ b/drivers/hid/hid-bpf.c
> > @@ -0,0 +1,176 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * BPF in HID support for Linux
> > + *
> > + * Copyright (c) 2021 Benjamin Tissoires
>
> Maybe 2022?

heh, maybe :)

>
> [...]
>
> > +static int hid_bpf_run_progs(struct hid_device *hdev, enum bpf_hid_attach_type type,
> > + struct hid_bpf_ctx *ctx, u8 *data, int size)
> > +{
> > + enum hid_bpf_event event = HID_BPF_UNDEF;
> > +
> > + if (type < 0 || !ctx)
> > + return -EINVAL;
> > +
> > + switch (type) {
> > + case BPF_HID_ATTACH_DEVICE_EVENT:
> > + event = HID_BPF_DEVICE_EVENT;
> > + if (size > sizeof(ctx->u.device.data))
> > + return -E2BIG;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + if (!hdev->bpf.run_array[type])
> > + return 0;
> > +
> > + memset(ctx, 0, sizeof(*ctx));
> > + ctx->hdev = hdev;
> > + ctx->type = event;
> > +
> > + if (size && data) {
> > + switch (event) {
> > + case HID_BPF_DEVICE_EVENT:
> > + memcpy(ctx->u.device.data, data, size);
> > + ctx->u.device.size = size;
> > + break;
> > + default:
> > + /* do nothing */
> > + }
> > + }
> > +
> > + BPF_PROG_RUN_ARRAY(hdev->bpf.run_array[type], ctx, bpf_prog_run);
>
> I guess we need "return BPF_PROG_RUN_ARRAY(...)"?

ack

>
> > +
> > + return 0;
> > +}
> > +
> > +u8 *hid_bpf_raw_event(struct hid_device *hdev, u8 *data, int *size)
> > +{
> > + int ret;
> > +
> > + if (bpf_hid_link_empty(&hdev->bpf, BPF_HID_ATTACH_DEVICE_EVENT))
> > + return data;
> > +
> > + ret = hid_bpf_run_progs(hdev, BPF_HID_ATTACH_DEVICE_EVENT,
> > + hdev->bpf.ctx, data, *size);
> > + if (ret)
> > + return data;
>
> shall we return ERR_PTR(ret)?

I initially wanted to have a bpf_program returning something other
than 0 being a signal to silently ignore the report.
But the API will be more consistent if we simply return an error in
the same way we do for bpf.ctx->u.device.size just below.

IOW, will change in v2

>
> > +
> > + if (!hdev->bpf.ctx->u.device.size)
> > + return ERR_PTR(-EINVAL);
> > +
> > + *size = hdev->bpf.ctx->u.device.size;
> > +
> > + return hdev->bpf.ctx->u.device.data;
> > +}
>
> [...]
>
> > diff --git a/include/uapi/linux/bpf_hid.h b/include/uapi/linux/bpf_hid.h
> > new file mode 100644
> > index 000000000000..243ac45a253f
> > --- /dev/null
> > +++ b/include/uapi/linux/bpf_hid.h
> > @@ -0,0 +1,39 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */
> > +
> > +/*
> > + * HID BPF public headers
> > + *
> > + * Copyright (c) 2021 Benjamin Tissoires
> > + */
> > +
> > +#ifndef _UAPI__LINUX_BPF_HID_H__
> > +#define _UAPI__LINUX_BPF_HID_H__
> > +
> > +#include <linux/types.h>
> > +
> > +#define HID_BPF_MAX_BUFFER_SIZE 16384 /* 16kb */
> > +
> > +struct hid_device;
> > +
> > +enum hid_bpf_event {
> > + HID_BPF_UNDEF = 0,
> > + HID_BPF_DEVICE_EVENT,
> > +};
> > +
> > +/* type is HID_BPF_DEVICE_EVENT */
> > +struct hid_bpf_ctx_device_event {
> > + __u8 data[HID_BPF_MAX_BUFFER_SIZE];
>
> 16kB sounds pretty big to me, do we usually need that much?

That's painful but it seems so: see commit 6a0eaf5123e0 ("HID:
Increase HID maximum report size to 16KB").

I wanted to have a static definition of the buffer, but maybe I could
terminate the struct with `_u8 data[]` and dynamically (re-)alloc the
buffer depending on the context.

If the verifier doesn't reject that (why would it? given that it
should rely on hid_is_valid_access()), I'll implement this in v2.

>
> > + unsigned long size;
> > +};
> > +
> > +struct hid_bpf_ctx {
> > + enum hid_bpf_event type;
> > + struct hid_device *hdev;
> > +
> > + union {
> > + struct hid_bpf_ctx_device_event device;
> > + } u;
> > +};
> > +
> > +#endif /* _UAPI__LINUX_BPF_HID_H__ */
> [...]
>
> > diff --git a/kernel/bpf/hid.c b/kernel/bpf/hid.c
> > new file mode 100644
> > index 000000000000..d3cb952bfc26
> > --- /dev/null
> > +++ b/kernel/bpf/hid.c
>
> [...]
>
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 9c7a72b65eee..230ca6964a7e 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3,6 +3,7 @@
> > */
> > #include <linux/bpf.h>
> > #include <linux/bpf-cgroup.h>
> > +#include <linux/bpf-hid.h>
> > #include <linux/bpf_trace.h>
> > #include <linux/bpf_lirc.h>
> > #include <linux/bpf_verifier.h>
> > @@ -2174,6 +2175,7 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type)
> > case BPF_PROG_TYPE_CGROUP_SYSCTL:
> > case BPF_PROG_TYPE_SOCK_OPS:
> > case BPF_PROG_TYPE_EXT: /* extends any prog */
> > + case BPF_PROG_TYPE_HID:
>
> Is this net_admin type?

Not really :)
I initially copied over from the LIRC2 code, which is something quite
similar in terms of abuse of BPF.

Maybe I should add an extra patch before introducing
is_sys_admin_prog_type() and move over LIRC2 there before adding HID.

>
> > return true;
> > case BPF_PROG_TYPE_CGROUP_SKB:
> > /* always unpriv */
> > @@ -3188,6 +3190,8 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
> > return BPF_PROG_TYPE_SK_LOOKUP;
> > case BPF_XDP:
> > return BPF_PROG_TYPE_XDP;
> > + case BPF_HID_DEVICE_EVENT:
> > + return BPF_PROG_TYPE_HID;
> > default:
> > return BPF_PROG_TYPE_UNSPEC;
> > }
> > @@ -3331,6 +3335,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
> > case BPF_SK_MSG_VERDICT:
> > case BPF_SK_SKB_VERDICT:
> > return sock_map_bpf_prog_query(attr, uattr);
> > + case BPF_HID_DEVICE_EVENT:
> > + return bpf_hid_prog_query(attr, uattr);
> > default:
> > return -EINVAL;
> > }
> > @@ -4325,6 +4331,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> > ret = bpf_perf_link_attach(attr, prog);
> > break;
> > #endif
> > + case BPF_PROG_TYPE_HID:
> > + return bpf_hid_link_create(attr, prog);
> > default:
> > ret = -EINVAL;
> > }
>
> [...]
>
> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index afe3d0d7f5f2..5978b92cacd3 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,7 @@ enum bpf_attach_type {
> > BPF_SK_REUSEPORT_SELECT,
> > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
> > BPF_PERF_EVENT,
> > + BPF_HID_DEVICE_EVENT,
> > __MAX_BPF_ATTACH_TYPE
> > };
> >
> > @@ -1011,6 +1013,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,
> > };
> > @@ -5870,6 +5873,10 @@ struct bpf_link_info {
> > struct {
> > __u32 ifindex;
> > } xdp;
> > + struct {
> > + __s32 hidraw_ino;
> > + __u32 attach_type;
> > + } hid;
> > };
> > } __attribute__((aligned(8)));
> >
>

And thanks for the initial review :)

Cheers,
Benjamin

2022-02-28 19:36:27

by Benjamin Tissoires

Subject: Re: [PATCH bpf-next v1 2/6] HID: bpf: allow to change the report descriptor from an eBPF program

On Sat, Feb 26, 2022 at 8:31 AM Song Liu <[email protected]> wrote:
>
> On Thu, Feb 24, 2022 at 3:09 AM Benjamin Tissoires
> <[email protected]> wrote:
> >
> > The report descriptor is the dictionary of the HID protocol specific
> > to the given device.
> > Changing it is a common habit in the HID world, and making that feature
> > accessible from eBPF allows to fix devices without having to install a
> > new kernel.
> >
> > Signed-off-by: Benjamin Tissoires <[email protected]>
>
> [...]
>
> > 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
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 5978b92cacd3..a7a8d9cfcf24 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -999,6 +999,7 @@ enum bpf_attach_type {
> > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
> > BPF_PERF_EVENT,
> > BPF_HID_DEVICE_EVENT,
> > + BPF_HID_RDESC_FIXUP,
> > __MAX_BPF_ATTACH_TYPE
> > };
> >
> > diff --git a/include/uapi/linux/bpf_hid.h b/include/uapi/linux/bpf_hid.h
> > index 243ac45a253f..c0801d7174c3 100644
> > --- a/include/uapi/linux/bpf_hid.h
> > +++ b/include/uapi/linux/bpf_hid.h
> > @@ -18,6 +18,7 @@ struct hid_device;
> > enum hid_bpf_event {
> > HID_BPF_UNDEF = 0,
> > HID_BPF_DEVICE_EVENT,
> > + HID_BPF_RDESC_FIXUP,
> > };
> >
> > /* type is HID_BPF_DEVICE_EVENT */
> > @@ -26,12 +27,19 @@ struct hid_bpf_ctx_device_event {
> > unsigned long size;
> > };
> >
> > +/* type is HID_BPF_RDESC_FIXUP */
> > +struct hid_bpf_ctx_rdesc_fixup {
> > + __u8 data[HID_BPF_MAX_BUFFER_SIZE];
> > + unsigned long size;
> > +};
>
> This looks same as HID_BPF_DEVICE_EVENT, do we really need to
> separate the two?

I wanted to separate them because the other types have other requirements.

However, they all need a "data" with "size" associated. So I'll add
data and size to the common definition of the struct, leaving only the
specifics in the union (which means that DEVICE_EVENT and RDESC_FIXUP
won't have a definition in the union). I'll see the look of it before
submitting v2.

Cheers,
Benjamin

2022-02-28 19:49:12

by Benjamin Tissoires

Subject: Re: [PATCH bpf-next v1 1/6] HID: initial BPF implementation

On Thu, Feb 24, 2022 at 12:41 PM Greg KH <[email protected]> wrote:
>
> On Thu, Feb 24, 2022 at 12:08:23PM +0100, Benjamin Tissoires wrote:
> > index 000000000000..243ac45a253f
> > --- /dev/null
> > +++ b/include/uapi/linux/bpf_hid.h
> > @@ -0,0 +1,39 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */
> > +
> > +/*
> > + * HID BPF public headers
> > + *
> > + * Copyright (c) 2021 Benjamin Tissoires
> > + */
> > +
> > +#ifndef _UAPI__LINUX_BPF_HID_H__
> > +#define _UAPI__LINUX_BPF_HID_H__
> > +
> > +#include <linux/types.h>
> > +
> > +#define HID_BPF_MAX_BUFFER_SIZE 16384 /* 16kb */
> > +
> > +struct hid_device;
> > +
> > +enum hid_bpf_event {
> > + HID_BPF_UNDEF = 0,
> > + HID_BPF_DEVICE_EVENT,
> > +};
> > +
> > +/* type is HID_BPF_DEVICE_EVENT */
> > +struct hid_bpf_ctx_device_event {
> > + __u8 data[HID_BPF_MAX_BUFFER_SIZE];
> > + unsigned long size;
>
> That's not a valid type to cross the user/kernel boundry, shouldn't it
> be "__u64"? But really, isn't __u32 enough here?

Fixed locally with a __u16 instead. Will be present in v2.

Cheers,
Benjamin