2022-07-12 15:14:30

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 00/23] Introduce eBPF support for HID devices

Hi,

and after a little bit of time, here comes the v6 of the HID-BPF series.

Again, for a full explanation of HID-BPF, please refer to the last patch
in this series (23/23).

This version sees some improvements compared to v5 on top of the
usual addressing of the previous comments:
- now I think every eBPF core change has a matching selftest added
- the kfuncs declared in syscall can now actually access the memory of
the context
- the code to retrieve the BTF ID of the various HID hooks is much
simpler (just a plain use of the BTF_ID() API instead of
loading/unloading of a tracing program)
- I also added my HID Surface Dial example that I use locally to provide
a fuller example to users

Cheers,
Benjamin

Benjamin Tissoires (23):
selftests/bpf: fix config for CLS_BPF
bpf/verifier: allow kfunc to read user provided context
bpf/verifier: do not clear meta in check_mem_size
selftests/bpf: add test for accessing ctx from syscall program type
bpf/verifier: allow kfunc to return an allocated mem
selftests/bpf: Add tests for kfunc returning a memory pointer
bpf: prepare for more bpf syscall to be used from kernel and user
space.
libbpf: add map_get_fd_by_id and map_delete_elem in light skeleton
HID: core: store the unique system identifier in hid_device
HID: export hid_report_type to uapi
HID: convert defines of HID class requests into a proper enum
HID: initial BPF implementation
selftests/bpf: add tests for the HID-bpf initial implementation
HID: bpf: allocate data memory for device_event BPF programs
selftests/bpf/hid: add test to change the report size
HID: bpf: introduce hid_hw_request()
selftests/bpf: add tests for bpf_hid_hw_request
HID: bpf: allow to change the report descriptor
selftests/bpf: add report descriptor fixup tests
selftests/bpf: Add a test for BPF_F_INSERT_HEAD
samples/bpf: add new hid_mouse example
HID: bpf: add Surface Dial example
Documentation: add HID-BPF docs

Documentation/hid/hid-bpf.rst | 512 +++++++++
Documentation/hid/index.rst | 1 +
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/bpf/Kconfig | 19 +
drivers/hid/bpf/Makefile | 11 +
drivers/hid/bpf/entrypoints/Makefile | 88 ++
drivers/hid/bpf/entrypoints/README | 4 +
drivers/hid/bpf/entrypoints/entrypoints.bpf.c | 66 ++
.../hid/bpf/entrypoints/entrypoints.lskel.h | 682 ++++++++++++
drivers/hid/bpf/hid_bpf_dispatch.c | 554 ++++++++++
drivers/hid/bpf/hid_bpf_dispatch.h | 28 +
drivers/hid/bpf/hid_bpf_jmp_table.c | 577 ++++++++++
drivers/hid/hid-core.c | 49 +-
include/linux/bpf.h | 10 +-
include/linux/btf.h | 14 +
include/linux/hid.h | 38 +-
include/linux/hid_bpf.h | 145 +++
include/uapi/linux/hid.h | 26 +-
include/uapi/linux/hid_bpf.h | 25 +
kernel/bpf/btf.c | 67 +-
kernel/bpf/syscall.c | 10 +-
kernel/bpf/verifier.c | 67 +-
net/bpf/test_run.c | 23 +
samples/bpf/.gitignore | 2 +
samples/bpf/Makefile | 27 +
samples/bpf/hid_mouse.bpf.c | 134 +++
samples/bpf/hid_mouse.c | 150 +++
samples/bpf/hid_surface_dial.bpf.c | 161 +++
samples/bpf/hid_surface_dial.c | 216 ++++
tools/include/uapi/linux/hid.h | 62 ++
tools/include/uapi/linux/hid_bpf.h | 25 +
tools/lib/bpf/skel_internal.h | 23 +
tools/testing/selftests/bpf/Makefile | 5 +-
tools/testing/selftests/bpf/config | 5 +-
tools/testing/selftests/bpf/prog_tests/hid.c | 990 ++++++++++++++++++
.../selftests/bpf/prog_tests/kfunc_call.c | 68 ++
tools/testing/selftests/bpf/progs/hid.c | 206 ++++
.../selftests/bpf/progs/kfunc_call_test.c | 116 ++
39 files changed, 5150 insertions(+), 60 deletions(-)
create mode 100644 Documentation/hid/hid-bpf.rst
create mode 100644 drivers/hid/bpf/Kconfig
create mode 100644 drivers/hid/bpf/Makefile
create mode 100644 drivers/hid/bpf/entrypoints/Makefile
create mode 100644 drivers/hid/bpf/entrypoints/README
create mode 100644 drivers/hid/bpf/entrypoints/entrypoints.bpf.c
create mode 100644 drivers/hid/bpf/entrypoints/entrypoints.lskel.h
create mode 100644 drivers/hid/bpf/hid_bpf_dispatch.c
create mode 100644 drivers/hid/bpf/hid_bpf_dispatch.h
create mode 100644 drivers/hid/bpf/hid_bpf_jmp_table.c
create mode 100644 include/linux/hid_bpf.h
create mode 100644 include/uapi/linux/hid_bpf.h
create mode 100644 samples/bpf/hid_mouse.bpf.c
create mode 100644 samples/bpf/hid_mouse.c
create mode 100644 samples/bpf/hid_surface_dial.bpf.c
create mode 100644 samples/bpf/hid_surface_dial.c
create mode 100644 tools/include/uapi/linux/hid.h
create mode 100644 tools/include/uapi/linux/hid_bpf.h
create mode 100644 tools/testing/selftests/bpf/prog_tests/hid.c
create mode 100644 tools/testing/selftests/bpf/progs/hid.c

--
2.36.1


2022-07-12 15:17:32

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 16/23] HID: bpf: introduce hid_hw_request()

This function can not be called under IRQ, thus it is only available
while in SEC("syscall").
For consistency, this function requires a HID-BPF context to work with,
and so we also provide a helper to create one based on the HID unique
ID.

Signed-off-by: Benjamin Tissoires <[email protected]>

--

changes in v6:
- rename parameter size into buf__sz to teach the verifier about
the actual buffer size used by the call
- remove the allocated data in the user created context, it's not used

new-ish in v5
---
drivers/hid/bpf/hid_bpf_dispatch.c | 148 +++++++++++++++++++++++++++++
drivers/hid/hid-core.c | 2 +
include/linux/hid_bpf.h | 13 ++-
3 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 87fd11539213..8348f5ae17f8 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -232,14 +232,162 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
return __hid_bpf_attach_prog(hdev, prog_type, prog_fd, flags);
}

+/**
+ * hid_bpf_allocate_context - Allocate a context to the given HID device
+ *
+ * @hid_id: the system unique identifier of the HID device
+ *
+ * @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error.
+ */
+noinline struct hid_bpf_ctx *
+hid_bpf_allocate_context(unsigned int hid_id)
+{
+ struct hid_device *hdev;
+ struct hid_bpf_ctx_kern *ctx_kern = NULL;
+ struct device *dev;
+ int err;
+
+ if (!hid_bpf_ops)
+ return NULL;
+
+ dev = bus_find_device(hid_bpf_ops->bus_type, NULL, &hid_id, device_match_id);
+ if (!dev)
+ return NULL;
+
+ hdev = to_hid_device(dev);
+
+ ctx_kern = kzalloc(sizeof(*ctx_kern), GFP_KERNEL);
+ if (!ctx_kern)
+ return NULL;
+
+ ctx_kern->ctx.hid = hdev;
+
+ return &ctx_kern->ctx;
+}
+
+/**
+ * hid_bpf_release_context - Release the previously allocated context @ctx
+ *
+ * @ctx: the HID-BPF context to release
+ *
+ */
+noinline void
+hid_bpf_release_context(struct hid_bpf_ctx *ctx)
+{
+ struct hid_bpf_ctx_kern *ctx_kern;
+
+ if (!ctx)
+ return;
+
+ ctx_kern = container_of(ctx, struct hid_bpf_ctx_kern, ctx);
+
+ kfree(ctx_kern);
+}
+
+/**
+ * hid_bpf_hw_request - Communicate with a HID device
+ *
+ * @ctx: the HID-BPF context previously allocated in hid_bpf_allocate_context()
+ * @buf: a %PTR_TO_MEM buffer
+ * @buf__sz: the size of the data to transfer
+ * @rtype: the type of the report (%HID_INPUT_REPORT, %HID_FEATURE_REPORT, %HID_OUTPUT_REPORT)
+ * @reqtype: the type of the request (%HID_REQ_GET_REPORT, %HID_REQ_SET_REPORT, ...)
+ *
+ * @returns %0 on success, a negative error code otherwise.
+ */
+noinline int
+hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
+ enum hid_report_type rtype, enum hid_class_request reqtype)
+{
+ struct hid_device *hdev = (struct hid_device *)ctx->hid; /* discard const */
+ struct hid_report *report;
+ struct hid_report_enum *report_enum;
+ u8 *dma_data;
+ u32 report_len;
+ int ret;
+
+ /* check arguments */
+ if (!ctx || !hid_bpf_ops)
+ return -EINVAL;
+
+ 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 (buf__sz < 1)
+ return -EINVAL;
+
+ report_enum = hdev->report_enum + rtype;
+ report = hid_bpf_ops->hid_get_report(report_enum, buf);
+ if (!report)
+ return -EINVAL;
+
+ report_len = hid_report_len(report);
+
+ if (buf__sz > report_len)
+ buf__sz = report_len;
+
+ dma_data = kmemdup(buf, buf__sz, GFP_KERNEL);
+ if (!dma_data)
+ return -ENOMEM;
+
+ ret = hid_bpf_ops->hid_hw_raw_request(hdev,
+ dma_data[0],
+ dma_data,
+ buf__sz,
+ rtype,
+ reqtype);
+
+ if (ret > 0)
+ memcpy(buf, dma_data, ret);
+
+ kfree(dma_data);
+ return ret;
+}
+
/* for syscall HID-BPF */
BTF_SET_START(hid_bpf_syscall_kfunc_ids)
BTF_ID(func, hid_bpf_attach_prog)
+BTF_ID(func, hid_bpf_allocate_context)
+BTF_ID(func, hid_bpf_release_context)
+BTF_ID(func, hid_bpf_hw_request)
BTF_SET_END(hid_bpf_syscall_kfunc_ids)

+BTF_SET_START(hid_bpf_syscall_kfunc_ret_null_ids)
+BTF_ID(func, hid_bpf_allocate_context)
+BTF_SET_END(hid_bpf_syscall_kfunc_ret_null_ids)
+
+BTF_SET_START(hid_bpf_syscall_kfunc_alloc_ids)
+BTF_ID(func, hid_bpf_allocate_context)
+BTF_SET_END(hid_bpf_syscall_kfunc_alloc_ids)
+
+BTF_SET_START(hid_bpf_syscall_kfunc_release_ids)
+BTF_ID(func, hid_bpf_release_context)
+BTF_SET_END(hid_bpf_syscall_kfunc_release_ids)
+
static const struct btf_kfunc_id_set hid_bpf_syscall_kfunc_set = {
.owner = THIS_MODULE,
.check_set = &hid_bpf_syscall_kfunc_ids,
+ .ret_null_set = &hid_bpf_syscall_kfunc_ret_null_ids,
+ .acquire_set = &hid_bpf_syscall_kfunc_alloc_ids,
+ .release_set = &hid_bpf_syscall_kfunc_release_ids,
};

int hid_bpf_connect_device(struct hid_device *hdev)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index c2589106ea4b..356d3822f17d 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2917,6 +2917,8 @@ int hid_check_keys_pressed(struct hid_device *hid)
EXPORT_SYMBOL_GPL(hid_check_keys_pressed);

static struct hid_bpf_ops hid_ops = {
+ .hid_get_report = hid_get_report,
+ .hid_hw_raw_request = hid_hw_raw_request,
.owner = THIS_MODULE,
.bus_type = &hid_bus_type,
};
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index c9684de18f3f..ade0def154b6 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -61,11 +61,15 @@ struct hid_bpf_ctx {
int hid_bpf_device_event(struct hid_bpf_ctx *ctx);

/* Following functions are kfunc that we export to BPF programs */
-/* only available in tracing */
+/* available everywhere in HID-BPF */
__u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t __sz);

/* only available in syscall */
int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags);
+int hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
+ enum hid_report_type rtype, enum hid_class_request reqtype);
+struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id);
+void hid_bpf_release_context(struct hid_bpf_ctx *ctx);

/*
* Below is HID internal
@@ -81,7 +85,14 @@ enum hid_bpf_prog_type {
HID_BPF_PROG_TYPE_MAX,
};

+struct hid_report_enum;
+
struct hid_bpf_ops {
+ struct hid_report *(*hid_get_report)(struct hid_report_enum *report_enum, const u8 *data);
+ int (*hid_hw_raw_request)(struct hid_device *hdev,
+ unsigned char reportnum, __u8 *buf,
+ size_t len, enum hid_report_type rtype,
+ enum hid_class_request reqtype);
struct module *owner;
struct bus_type *bus_type;
};
--
2.36.1

2022-07-12 15:18:54

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 20/23] selftests/bpf: Add a test for BPF_F_INSERT_HEAD

Insert 3 programs to check that we are doing the correct thing:
'2', '1', '3' are inserted, but '1' is supposed to be executed first.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

changes in v6:
- fixed copy/paste in ASSERT_OK and test execution order

changes in v5:
- use the new API

not in v4

changes in v3:
- use the new hid_get_data API

new in v2
---
tools/testing/selftests/bpf/prog_tests/hid.c | 107 +++++++++++++++++++
tools/testing/selftests/bpf/progs/hid.c | 54 +++++++++-
2 files changed, 160 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index 9dc5f0038472..a86e16554e68 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/hid_bpf.h>
#include <linux/hidraw.h>
#include <linux/uhid.h>

@@ -83,6 +84,7 @@ static u8 feature_data[] = { 1, 2 };
struct attach_prog_args {
int prog_fd;
unsigned int hid;
+ unsigned int flags;
int retval;
};

@@ -770,6 +772,109 @@ static int test_hid_user_raw_request_call(int uhid_fd, int dev_id)
return ret;
}

+/*
+ * Attach hid_insert{0,1,2} to the given uhid device,
+ * retrieve and open the matching hidraw node,
+ * inject one event in the uhid device,
+ * check that the programs have been inserted in the correct order.
+ */
+static int test_hid_attach_flags(int uhid_fd, int dev_id)
+{
+ struct hid *hid_skel = NULL;
+ u8 buf[64] = {0};
+ int hidraw_fd = -1;
+ int hid_id, attach_fd, err = -EINVAL;
+ struct attach_prog_args args = {
+ .retval = -1,
+ };
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattr,
+ .ctx_in = &args,
+ .ctx_size_in = sizeof(args),
+ );
+
+ /* locate the uevent file of the created device */
+ hid_id = get_hid_id(dev_id);
+ if (!ASSERT_GE(hid_id, 0, "locate uhid device id"))
+ goto cleanup;
+
+ args.hid = hid_id;
+
+ hid_skel = hid__open();
+ if (!ASSERT_OK_PTR(hid_skel, "hid_skel_open"))
+ goto cleanup;
+
+ bpf_program__set_autoload(hid_skel->progs.hid_test_insert1, true);
+ bpf_program__set_autoload(hid_skel->progs.hid_test_insert2, true);
+ bpf_program__set_autoload(hid_skel->progs.hid_test_insert3, true);
+
+ err = hid__load(hid_skel);
+ if (!ASSERT_OK(err, "hid_skel_load"))
+ goto cleanup;
+
+ attach_fd = bpf_program__fd(hid_skel->progs.attach_prog);
+ if (!ASSERT_GE(attach_fd, 0, "locate attach_prog")) {
+ err = attach_fd;
+ goto cleanup;
+ }
+
+ /* attach hid_test_insert2 program */
+ args.prog_fd = bpf_program__fd(hid_skel->progs.hid_test_insert2);
+ args.flags = HID_BPF_FLAG_NONE;
+ args.retval = 1;
+ err = bpf_prog_test_run_opts(attach_fd, &tattr);
+ if (!ASSERT_EQ(args.retval, 0, "attach_hid_test_insert2"))
+ goto cleanup;
+
+ /* then attach hid_test_insert1 program before the previous*/
+ args.prog_fd = bpf_program__fd(hid_skel->progs.hid_test_insert1);
+ args.flags = HID_BPF_FLAG_INSERT_HEAD;
+ args.retval = 1;
+ err = bpf_prog_test_run_opts(attach_fd, &tattr);
+ if (!ASSERT_EQ(args.retval, 0, "attach_hid_test_insert1"))
+ goto cleanup;
+
+ /* finally attach hid_test_insert3 at the end */
+ args.prog_fd = bpf_program__fd(hid_skel->progs.hid_test_insert3);
+ args.flags = HID_BPF_FLAG_NONE;
+ args.retval = 1;
+ err = bpf_prog_test_run_opts(attach_fd, &tattr);
+ if (!ASSERT_EQ(args.retval, 0, "attach_hid_test_insert3"))
+ goto cleanup;
+
+ hidraw_fd = open_hidraw(dev_id);
+ if (!ASSERT_GE(hidraw_fd, 0, "open_hidraw"))
+ goto cleanup;
+
+ /* inject one event */
+ buf[0] = 1;
+ send_event(uhid_fd, buf, 6);
+
+ /* read the data from hidraw */
+ memset(buf, 0, sizeof(buf));
+ err = read(hidraw_fd, buf, sizeof(buf));
+ if (!ASSERT_EQ(err, 6, "read_hidraw"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[1], 1, "hid_test_insert1"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[2], 2, "hid_test_insert2"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[3], 3, "hid_test_insert3"))
+ goto cleanup;
+
+ err = 0;
+
+ cleanup:
+ if (hidraw_fd >= 0)
+ close(hidraw_fd);
+
+ hid__destroy(hid_skel);
+
+ return err;
+}
+
/*
* Attach hid_rdesc_fixup to the given uhid device,
* retrieve and open the matching hidraw node,
@@ -866,6 +971,8 @@ void serial_test_hid_bpf(void)
ASSERT_OK(err, "hid_change_report");
err = test_hid_user_raw_request_call(uhid_fd, dev_id);
ASSERT_OK(err, "hid_user_raw_request");
+ err = test_hid_attach_flags(uhid_fd, dev_id);
+ ASSERT_OK(err, "hid_attach_flags");

/*
* this test should be run last because we disconnect/reconnect
diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
index 863c37ddf5ff..92cf57d2be3e 100644
--- a/tools/testing/selftests/bpf/progs/hid.c
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -21,6 +21,7 @@ extern int hid_bpf_hw_request(struct hid_bpf_ctx *ctx,
struct attach_prog_args {
int prog_fd;
unsigned int hid;
+ unsigned int flags;
int retval;
};

@@ -60,7 +61,7 @@ int attach_prog(struct attach_prog_args *ctx)
{
ctx->retval = hid_bpf_attach_prog(ctx->hid,
ctx->prog_fd,
- 0);
+ ctx->flags);
return 0;
}

@@ -152,3 +153,54 @@ int BPF_PROG(hid_rdesc_fixup, struct hid_bpf_ctx *hid_ctx)

return sizeof(rdesc) + 73;
}
+
+SEC("?fmod_ret/hid_bpf_device_event")
+int BPF_PROG(hid_test_insert1, struct hid_bpf_ctx *hid_ctx)
+{
+ __u8 *data = hid_bpf_get_data(hid_ctx, 0 /* offset */, 4 /* size */);
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ /* we need to be run first */
+ if (data[2] || data[3])
+ return -1;
+
+ data[1] = 1;
+
+ return 0;
+}
+
+SEC("?fmod_ret/hid_bpf_device_event")
+int BPF_PROG(hid_test_insert2, struct hid_bpf_ctx *hid_ctx)
+{
+ __u8 *data = hid_bpf_get_data(hid_ctx, 0 /* offset */, 4 /* size */);
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ /* after insert0 and before insert2 */
+ if (!data[1] || data[3])
+ return -1;
+
+ data[2] = 2;
+
+ return 0;
+}
+
+SEC("?fmod_ret/hid_bpf_device_event")
+int BPF_PROG(hid_test_insert3, struct hid_bpf_ctx *hid_ctx)
+{
+ __u8 *data = hid_bpf_get_data(hid_ctx, 0 /* offset */, 4 /* size */);
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ /* at the end */
+ if (!data[1] || !data[2])
+ return -1;
+
+ data[3] = 3;
+
+ return 0;
+}
--
2.36.1

2022-07-12 15:19:03

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 23/23] Documentation: add HID-BPF docs

Gives a primer on HID-BPF.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

changes in v6:
- amended the example now that we can directly use the data from the
syscall context

changes in v5:
- amended for new API
- reworded most of the sentences (thanks to Peter Hutterer for the review)

changes in v4:
- fixed typos

new in v3
---
Documentation/hid/hid-bpf.rst | 512 ++++++++++++++++++++++++++++++++++
Documentation/hid/index.rst | 1 +
2 files changed, 513 insertions(+)
create mode 100644 Documentation/hid/hid-bpf.rst

diff --git a/Documentation/hid/hid-bpf.rst b/Documentation/hid/hid-bpf.rst
new file mode 100644
index 000000000000..75e65c135925
--- /dev/null
+++ b/Documentation/hid/hid-bpf.rst
@@ -0,0 +1,512 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=======
+HID-BPF
+=======
+
+HID is a standard protocol for input devices but some devices may require
+custom tweaks, traditionally done with a kernel driver fix. Using the eBPF
+capabilities instead speeds up development and adds new capabilities to the
+existing HID interfaces.
+
+.. contents::
+ :local:
+ :depth: 2
+
+
+When (and why) to use HID-BPF
+=============================
+
+We can enumerate several use cases for when using HID-BPF is better than
+using a standard kernel driver fix:
+
+Dead zone of a joystick
+-----------------------
+
+Assuming you have a joystick that is getting older, it is common to see it
+wobbling around its neutral point. This is usually filtered at the application
+level by adding a *dead zone* for this specific axis.
+
+With HID-BPF, we can apply this filtering in the kernel directly so userspace
+does not get woken up when nothing else is happening on the input controller.
+
+Of course, given that this dead zone is specific to an individual device, we
+can not create a generic fix for all of the same joysticks. Adding a custom
+kernel API for this (e.g. by adding a sysfs entry) does not guarantee this new
+kernel API will be broadly adopted and maintained.
+
+HID-BPF allows the userspace program to load the program itself, ensuring we
+only load the custom API when we have a user.
+
+Simple fixup of report descriptor
+---------------------------------
+
+In the HID tree, half of the drivers only fix one key or one byte
+in the report descriptor. These fixes all require a kernel patch and the
+subsequent shepherding into a release, a long and painful process for users.
+
+We can reduce this burden by providing an eBPF program instead. Once such a
+program has been verified by the user, we can embed the source code into the
+kernel tree and ship the eBPF program and load it directly instead of loading
+a specific kernel module for it.
+
+Note: distribution of eBPF programs and their inclusion in the kernel is not
+yet fully implemented
+
+Add a new feature that requires a new kernel API
+------------------------------------------------
+
+An example for such a feature are the Universal Stylus Interface (USI) pens.
+Basically, USI pens require a new kernel API because there are new
+channels of communication that our HID and input stack do not support.
+Instead of using hidraw or creating new sysfs entries 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.
+
+Morph a device into something else and control that from userspace
+------------------------------------------------------------------
+
+The kernel has a relatively static mapping of HID items to evdev bits.
+It cannot decide to dynamically transform a given device into something else
+as it does not have the required context and any such transformation cannot be
+undone (or even discovered) by userspace.
+
+However, some devices are useless with that static way of defining devices. For
+example, the Microsoft Surface Dial is a pushbutton with haptic feedback that
+is barely usable as of today.
+
+With eBPF, userspace can morph that device into a mouse, and convert the dial
+events into wheel events. Also, the userspace program can set/unset the haptic
+feedback depending on the context. For example, if a menu is visible on the
+screen we likely need to have a haptic click every 15 degrees. But when
+scrolling in a web page the user experience is better when the device emits
+events at the highest resolution.
+
+Firewall
+--------
+
+What if we want to prevent other users to access a specific feature of a
+device? (think a possibly broken firmware update entry point)
+
+With eBPF, we can intercept any HID command emitted to the device and
+validate it or not.
+
+This also allows to sync the state between the userspace and the
+kernel/bpf program because we can intercept any incoming command.
+
+Tracing
+-------
+
+The last usage 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 this may raise some eyebrows)
+2. hidraw doesn't catch other processes' requests to the device, which
+ means that we have cases where we need to add printks to the kernel
+ to understand what is happening.
+
+High-level view of HID-BPF
+==========================
+
+The main idea behind HID-BPF is that it works at an array of bytes level.
+Thus, all of the parsing of the HID report and the HID report descriptor
+must be implemented in the userspace component that loads the eBPF
+program.
+
+For example, in the dead zone joystick from above, knowing which fields
+in the data stream needs to be set to ``0`` needs to be computed by userspace.
+
+A corollary of this is that HID-BPF doesn't know about the other subsystems
+available in the kernel. *You can not directly emit input event through the
+input API from eBPF*.
+
+When a BPF program needs to emit input events, it needs to talk HID, and rely
+on the HID kernel processing to translate the HID data into input events.
+
+Available types of programs
+===========================
+
+HID-BPF is built "on top" of BPF, meaning that we use tracing method to
+declare our programs.
+
+HID-BPF has the following attachment types available:
+
+1. event processing/filtering with ``SEC("fmod_ret/hid_bpf_device_event")`` in libbpf
+2. actions coming from userspace with ``SEC("syscall")`` in libbpf
+3. change of the report descriptor with ``SEC("fmod_ret/hid_bpf_rdesc_fixup")`` in libbpf
+
+A ``hid_bpf_device_event`` is calling a BPF program when an event is received from
+the device. Thus we are in IRQ context and can act on the data or notify userspace.
+And given that we are in IRQ context, we can not talk back to the device.
+
+A ``syscall`` means that userspace called the syscall ``BPF_PROG_RUN`` facility.
+This time, we can do any operations allowed by HID-BPF, and talking to the device is
+allowed.
+
+Last, ``hid_bpf_rdesc_fixup`` is different from the others as there can be only one
+BPF program of this type. This is called on ``probe`` from the driver and allows to
+change the report descriptor from the BPF program. Once a ``hid_bpf_rdesc_fixup``
+program has been loaded, it is not possible to overwrite it unless the program which
+inserted it allows us by pinning the program and closing all of its fds pointing to it.
+
+Developer API:
+==============
+
+User API data structures available in programs:
+-----------------------------------------------
+
+.. kernel-doc:: include/uapi/linux/hid_bpf.h
+.. kernel-doc:: include/linux/hid_bpf.h
+
+Available tracing functions to attach a HID-BPF program:
+--------------------------------------------------------
+
+.. kernel-doc:: drivers/hid/bpf/hid_bpf_dispatch.c
+ :functions: hid_bpf_device_event hid_bpf_rdesc_fixup
+
+Available API that can be used in all HID-BPF programs:
+-------------------------------------------------------
+
+.. kernel-doc:: drivers/hid/bpf/hid_bpf_dispatch.c
+ :functions: hid_bpf_get_data
+
+Available API that can be used in syscall HID-BPF programs:
+-----------------------------------------------------------
+
+.. kernel-doc:: drivers/hid/bpf/hid_bpf_dispatch.c
+ :functions: hid_bpf_attach_prog hid_bpf_hw_request hid_bpf_allocate_context hid_bpf_release_context
+
+General overview of a HID-BPF program
+=====================================
+
+Accessing the data attached to the context
+------------------------------------------
+
+The ``struct hid_bpf_ctx`` doesn't export the ``data`` fields directly and to access
+it, a bpf program needs to first call :c:func:`hid_bpf_get_data`.
+
+``offset`` can be any integer, but ``size`` needs to be constant, known at compile
+time.
+
+This allows the following:
+
+1. for a given device, if we know that the report length will always be of a certain value,
+ we can request the ``data`` pointer to point at the full report length.
+
+ The kernel will ensure we are using a correct size and offset and eBPF will ensure
+ the code will not attempt to read or write outside of the boundaries::
+
+ __u8 *data = hid_bpf_get_data(ctx, 0 /* offset */, 256 /* size */);
+
+ if (!data)
+ return 0; /* ensure data is correct, now the verifier knows we
+ * have 256 bytes available */
+
+ bpf_printk("hello world: %02x %02x %02x", data[0], data[128], data[255]);
+
+2. if the report length is variable, but we know the value of ``X`` is always a 16-bit
+ integer, we can then have a pointer to that value only::
+
+ __u16 *x = hid_bpf_get_data(ctx, offset, sizeof(*x));
+
+ if (!x)
+ return 0; /* something went wrong */
+
+ *x += 1; /* increment X by one */
+
+Effect of a HID-BPF program
+---------------------------
+
+For all HID-BPF attachment types except for :c:func:`hid_bpf_rdesc_fixup`, several eBPF
+programs can be attached to the same device.
+
+Unless ``HID_BPF_FLAG_INSERT_HEAD`` is added to the flags while attaching the
+program, the new program is appended at the end of the list.
+``HID_BPF_FLAG_INSERT_HEAD`` will insert the new program at the beginning of the
+list which is useful for e.g. tracing where we need to get the unprocessed events
+from the device.
+
+Note that if there are multiple programs using the ``HID_BPF_FLAG_INSERT_HEAD`` flag,
+only the most recently loaded one is actually the first in the list.
+
+``SEC("fmod_ret/hid_bpf_device_event")``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Whenever a matching event is raised, the eBPF programs are called one after the other
+and are working on the same data buffer.
+
+If a program changes the data associated with the context, the next one will see
+the modified data but it will have *no* idea of what the original data was.
+
+Once all the programs are run and return ``0`` or a positive value, the rest of the
+HID stack will work on the modified data, with the ``size`` field of the last hid_bpf_ctx
+being the new size of the input stream of data.
+
+A BPF program returning a negative error discards the event, i.e. this event will not be
+processed by the HID stack. Clients (hidraw, input, LEDs) will **not** see this event.
+
+``SEC("syscall")``
+~~~~~~~~~~~~~~~~~~
+
+``syscall`` are not attached to a given device. To tell which device we are working
+with, userspace needs to refer to the device by its unique system id (the last 4 numbers
+in the sysfs path: ``/sys/bus/hid/devices/xxxx:yyyy:zzzz:0000``).
+
+To retrieve a context associated with the device, the program must call
+:c:func:`hid_bpf_allocate_context` and must release it with :c:func:`hid_bpf_release_context`
+before returning.
+Once the context is retrieved, one can also request a pointer to kernel memory with
+:c:func:`hid_bpf_get_data`. This memory is big enough to support all input/output/feature
+reports of the given device.
+
+``SEC("fmod_ret/hid_bpf_rdesc_fixup")``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The ``hid_bpf_rdesc_fixup`` program works in a similar manner to
+``.report_fixup`` of ``struct hid_driver``.
+
+When the device is probed, the kernel sets the data buffer of the context with the
+content of the report descriptor. The memory associated with that buffer is
+``HID_MAX_DESCRIPTOR_SIZE`` (currently 4kB).
+
+The eBPF program can modify the data buffer at-will and the kernel uses the
+modified content and size as the report descriptor.
+
+Whenever a ``SEC("fmod_ret/hid_bpf_rdesc_fixup")`` program is attached (if no
+program was attached before), the kernel immediately disconnects the HID device
+and does a reprobe.
+
+In the same way, when the ``SEC("fmod_ret/hid_bpf_rdesc_fixup")`` program is
+detached, the kernel issues a disconnect on the device.
+
+There is no ``detach`` facility in HID-BPF. Detaching a program happens when
+all the user space file descriptors pointing at a program are closed.
+Thus, if we need to replace a report descriptor fixup, some cooperation is
+required from the owner of the original report descriptor fixup.
+The previous owner will likely pin the program in the bpffs, and we can then
+replace it through normal bpf operations.
+
+Attaching a bpf program to a device
+===================================
+
+``libbpf`` does not export any helper to attach a HID-BPF program.
+Users need to use a dedicated ``syscall`` program which will call
+``hid_bpf_attach_prog(hid_id, program_fd, flags)``.
+
+``hid_id`` is the unique system ID of the HID device (the last 4 numbers in the
+sysfs path: ``/sys/bus/hid/devices/xxxx:yyyy:zzzz:0000``)
+
+``progam_fd`` is the opened file descriptor of the program to attach.
+
+``flags`` is of type ``enum hid_bpf_attach_flags``.
+
+We can not rely on hidraw to bind a BPF program to a HID device. hidraw is an
+artefact of the processing of the HID device, and is not stable. Some drivers
+even disable it, so that removes the tracing capabilies on those devices
+(where it is interesting to get the non-hidraw traces).
+
+On the other hand, the ``hid_id`` is stable for the entire life of the HID device,
+even if we change its report descriptor.
+
+Given that hidraw is not stable when the device disconnects/reconnects, we recommend
+accessing the current report descriptor of the device through the sysfs.
+This is available at ``/sys/bus/hid/devices/BUS:VID:PID.000N/report_descriptor`` as a
+binary stream.
+
+Parsing the report descriptor is the responsibility of the BPF programmer or the userspace
+component that loads the eBPF program.
+
+An (almost) complete example of a BPF enhanced HID device
+=========================================================
+
+*Foreword: for most parts, this could be implemented as a kernel driver*
+
+Let's imagine we have a new tablet device that has some haptic capabilities
+to simulate the surface the user is scratching on. This device would also have
+a specific 3 positions switch to toggle between *pencil on paper*, *cray on a wall*
+and *brush on a painting canvas*. To make things even better, we can control the
+physical position of the switch through a feature report.
+
+And of course, the switch is relying on some userspace component to control the
+haptic feature of the device itself.
+
+Filtering events
+----------------
+
+The first step consists in filtering events from the device. Given that the switch
+position is actually reported in the flow of the pen events, using hidraw to implement
+that filtering would mean that we wake up userspace for every single event.
+
+This is OK for libinput, but having an external library that is just interested in
+one byte in the report is less than ideal.
+
+For that, we can create a basic skeleton for our BPF program::
+
+ #include "vmlinux.h"
+ #include <bpf/bpf_helpers.h>
+ #include <bpf/bpf_tracing.h>
+
+ /* HID programs need to be GPL */
+ char _license[] SEC("license") = "GPL";
+
+ /* HID-BPF kfunc API definitions */
+ extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
+ unsigned int offset,
+ const size_t __sz) __ksym;
+ extern int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, u32 flags) __ksym;
+
+ struct {
+ __uint(type, BPF_MAP_TYPE_RINGBUF);
+ __uint(max_entries, 4096 * 64);
+ } ringbuf SEC(".maps");
+
+ struct attach_prog_args {
+ int prog_fd;
+ unsigned int hid;
+ unsigned int flags;
+ int retval;
+ };
+
+ SEC("syscall")
+ int attach_prog(struct attach_prog_args *ctx)
+ {
+ ctx->retval = hid_bpf_attach_prog(ctx->hid,
+ ctx->prog_fd,
+ ctx->flags);
+ return 0;
+ }
+
+ __u8 current_value = 0;
+
+ SEC("?fmod_ret/hid_bpf_device_event")
+ int BPF_PROG(filter_switch, struct hid_bpf_ctx *hid_ctx)
+ {
+ __u8 *data = hid_bpf_get_data(hid_ctx, 0 /* offset */, 192 /* size */);
+ __u8 *buf;
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ if (current_value != data[152]) {
+ buf = bpf_ringbuf_reserve(&ringbuf, 1, 0);
+ if (!buf)
+ return 0;
+
+ *buf = data[152];
+
+ bpf_ringbuf_commit(buf, 0);
+
+ current_value = data[152];
+ }
+
+ return 0;
+ }
+
+To attach ``filter_switch``, userspace needs to call the ``attach_prog`` syscall
+program first::
+
+ static int attach_filter(struct hid *hid_skel, int hid_id)
+ {
+ int err, prog_fd;
+ int ret = -1;
+ struct attach_prog_args args = {
+ .hid = hid_id,
+ };
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattrs,
+ .ctx_in = &args,
+ .ctx_size_in = sizeof(args),
+ );
+
+ args.prog_fd = bpf_program__fd(hid_skel->progs.filter_switch);
+
+ prog_fd = bpf_program__fd(hid_skel->progs.attach_prog);
+
+ err = bpf_prog_test_run_opts(prog_fd, &tattrs);
+ return err;
+ }
+
+Our userspace program can now listen to notifications on the ring buffer, and
+is awaken only when the value changes.
+
+Controlling the device
+----------------------
+
+To be able to change the haptic feedback from the tablet, the userspace program
+needs to emit a feature report on the device itself.
+
+Instead of using hidraw for that, we can create a ``SEC("syscall")`` program
+that talks to the device::
+
+ /* some more HID-BPF kfunc API definitions */
+ extern struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id) __ksym;
+ extern void hid_bpf_release_context(struct hid_bpf_ctx *ctx) __ksym;
+ extern int hid_bpf_hw_request(struct hid_bpf_ctx *ctx,
+ __u8* data,
+ size_t len,
+ enum hid_report_type type,
+ enum hid_class_request reqtype) __ksym;
+
+
+ struct hid_send_haptics_args {
+ /* data needs to come at offset 0 so we can do a memcpy into it */
+ __u8 data[10];
+ unsigned int hid;
+ };
+
+ SEC("syscall")
+ int send_haptic(struct hid_send_haptics_args *args)
+ {
+ struct hid_bpf_ctx *ctx;
+ int ret = 0;
+
+ ctx = hid_bpf_allocate_context(args->hid);
+ if (!ctx)
+ return 0; /* EPERM check */
+
+ ret = hid_bpf_hw_request(ctx,
+ args->data,
+ 10,
+ HID_FEATURE_REPORT,
+ HID_REQ_SET_REPORT);
+
+ hid_bpf_release_context(ctx);
+
+ return ret;
+ }
+
+And then userspace needs to call that program directly::
+
+ static int set_haptic(struct hid *hid_skel, int hid_id, __u8 haptic_value)
+ {
+ int err, prog_fd;
+ int ret = -1;
+ struct hid_send_haptics_args args = {
+ .hid = hid_id,
+ };
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattrs,
+ .ctx_in = &args,
+ .ctx_size_in = sizeof(args),
+ );
+
+ args.data[0] = 0x02; /* report ID of the feature on our device */
+ args.data[1] = haptic_value;
+
+ prog_fd = bpf_program__fd(hid_skel->progs.set_haptic);
+
+ err = bpf_prog_test_run_opts(prog_fd, &tattrs);
+ return err;
+ }
+
+Now our userspace program is aware of the haptic state and can control it. The
+program could make this state further available to other userspace programs
+(e.g. via a DBus API).
+
+The interesting bit here is that we did not created a new kernel API for this.
+Which means that if there is a bug in our implementation, we can change the
+interface with the kernel at-will, because the userspace application is
+responsible for its own usage.
diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst
index e50f513c579c..b2028f382f11 100644
--- a/Documentation/hid/index.rst
+++ b/Documentation/hid/index.rst
@@ -11,6 +11,7 @@ Human Interface Devices (HID)
hidraw
hid-sensor
hid-transport
+ hid-bpf

uhid

--
2.36.1

2022-07-12 15:19:41

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 17/23] selftests/bpf: add tests for bpf_hid_hw_request

Add tests for the newly implemented function.
We test here only the GET_REPORT part because the other calls are pure
HID protocol and won't infer the result of the test of the bpf hook.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

changes in v6:
- fixed copy/paste in prog_tests when calling ASSERT_OK
- removed the need for memcpy now that kfuncs can access ctx

changes in v5:
- use the new hid_bpf_allocate_context() API
- remove the need for ctx_in for syscall TEST_RUN

changes in v3:
- use the new hid_get_data API
- directly use HID_FEATURE_REPORT and HID_REQ_GET_REPORT from uapi

changes in v2:
- split the series by bpf/libbpf/hid/selftests and samples
---
tools/testing/selftests/bpf/prog_tests/hid.c | 114 ++++++++++++++++---
tools/testing/selftests/bpf/progs/hid.c | 43 +++++++
2 files changed, 139 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index 47bc0a30c275..19172d3e0f44 100644
--- a/tools/testing/selftests/bpf/prog_tests/hid.c
+++ b/tools/testing/selftests/bpf/prog_tests/hid.c
@@ -77,12 +77,23 @@ static unsigned char rdesc[] = {
0xc0, /* END_COLLECTION */
};

+static u8 feature_data[] = { 1, 2 };
+
struct attach_prog_args {
int prog_fd;
unsigned int hid;
int retval;
};

+struct hid_hw_request_syscall_args {
+ __u8 data[10];
+ unsigned int hid;
+ int retval;
+ size_t size;
+ enum hid_report_type type;
+ __u8 request_type;
+};
+
static pthread_mutex_t uhid_started_mtx = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t uhid_started = PTHREAD_COND_INITIALIZER;

@@ -142,7 +153,7 @@ static void destroy(int fd)

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

memset(&ev, 0, sizeof(ev));
@@ -183,6 +194,15 @@ static int uhid_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");
@@ -391,6 +411,7 @@ static int open_hidraw(int dev_id)
struct test_params {
struct hid *skel;
int hidraw_fd;
+ int hid_id;
};

static int prep_test(int dev_id, const char *prog_name, struct test_params *test_data)
@@ -419,27 +440,33 @@ static int prep_test(int dev_id, const char *prog_name, struct test_params *test
if (!ASSERT_OK_PTR(hid_skel, "hid_skel_open"))
goto cleanup;

- prog = bpf_object__find_program_by_name(*hid_skel->skeleton->obj, prog_name);
- if (!ASSERT_OK_PTR(prog, "find_prog_by_name"))
- goto cleanup;
+ if (prog_name) {
+ prog = bpf_object__find_program_by_name(*hid_skel->skeleton->obj, prog_name);
+ if (!ASSERT_OK_PTR(prog, "find_prog_by_name"))
+ goto cleanup;

- bpf_program__set_autoload(prog, true);
+ bpf_program__set_autoload(prog, true);

- err = hid__load(hid_skel);
- if (!ASSERT_OK(err, "hid_skel_load"))
- goto cleanup;
+ err = hid__load(hid_skel);
+ if (!ASSERT_OK(err, "hid_skel_load"))
+ goto cleanup;

- attach_fd = bpf_program__fd(hid_skel->progs.attach_prog);
- if (!ASSERT_GE(attach_fd, 0, "locate attach_prog")) {
- err = attach_fd;
- goto cleanup;
- }
+ attach_fd = bpf_program__fd(hid_skel->progs.attach_prog);
+ if (!ASSERT_GE(attach_fd, 0, "locate attach_prog")) {
+ err = attach_fd;
+ goto cleanup;
+ }

- args.prog_fd = bpf_program__fd(prog);
- err = bpf_prog_test_run_opts(attach_fd, &tattr);
- snprintf(buf, sizeof(buf), "attach_hid(%s)", prog_name);
- if (!ASSERT_EQ(args.retval, 0, buf))
- goto cleanup;
+ args.prog_fd = bpf_program__fd(prog);
+ err = bpf_prog_test_run_opts(attach_fd, &tattr);
+ snprintf(buf, sizeof(buf), "attach_hid(%s)", prog_name);
+ if (!ASSERT_EQ(args.retval, 0, buf))
+ goto cleanup;
+ } else {
+ err = hid__load(hid_skel);
+ if (!ASSERT_OK(err, "hid_skel_load"))
+ goto cleanup;
+ }

hidraw_fd = open_hidraw(dev_id);
if (!ASSERT_GE(hidraw_fd, 0, "open_hidraw"))
@@ -447,6 +474,7 @@ static int prep_test(int dev_id, const char *prog_name, struct test_params *test

test_data->skel = hid_skel;
test_data->hidraw_fd = hidraw_fd;
+ test_data->hid_id = hid_id;

return 0;

@@ -693,6 +721,54 @@ static int test_hid_change_report(int uhid_fd, int dev_id)
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(int uhid_fd, int dev_id)
+{
+ struct test_params params;
+ int err, prog_fd;
+ int ret = -1;
+ struct hid_hw_request_syscall_args args = {
+ .retval = -1,
+ .type = HID_FEATURE_REPORT,
+ .request_type = HID_REQ_GET_REPORT,
+ .size = 10,
+ };
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattrs,
+ .ctx_in = &args,
+ .ctx_size_in = sizeof(args),
+ );
+
+ err = prep_test(dev_id, NULL, &params);
+ if (!ASSERT_EQ(err, 0, "prep_test()"))
+ goto cleanup;
+
+ args.hid = params.hid_id;
+ args.data[0] = 1; /* report ID */
+
+ prog_fd = bpf_program__fd(params.skel->progs.hid_user_raw_request);
+
+ err = bpf_prog_test_run_opts(prog_fd, &tattrs);
+ if (!ASSERT_EQ(err, 0, "bpf_prog_test_run_opts"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(args.retval, 2, "bpf_prog_test_run_opts_retval"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(args.data[1], 2, "hid_user_raw_request_check_in"))
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ cleanup_test(&params);
+
+ return ret;
+}
+
void serial_test_hid_bpf(void)
{
int err, uhid_fd;
@@ -720,6 +796,8 @@ void serial_test_hid_bpf(void)
ASSERT_OK(err, "hid_attach_detach");
err = test_hid_change_report(uhid_fd, dev_id);
ASSERT_OK(err, "hid_change_report");
+ err = test_hid_user_raw_request_call(uhid_fd, dev_id);
+ ASSERT_OK(err, "hid_user_raw_request");

destroy(uhid_fd);

diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
index ee7529c47ad8..0be319c11575 100644
--- a/tools/testing/selftests/bpf/progs/hid.c
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -10,6 +10,13 @@ extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
unsigned int offset,
const size_t __sz) __ksym;
extern int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, u32 flags) __ksym;
+extern struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id) __ksym;
+extern void hid_bpf_release_context(struct hid_bpf_ctx *ctx) __ksym;
+extern int hid_bpf_hw_request(struct hid_bpf_ctx *ctx,
+ __u8 *data,
+ size_t buf__sz,
+ enum hid_report_type type,
+ enum hid_class_request reqtype) __ksym;

struct attach_prog_args {
int prog_fd;
@@ -56,3 +63,39 @@ int attach_prog(struct attach_prog_args *ctx)
0);
return 0;
}
+
+struct hid_hw_request_syscall_args {
+ /* data needs to come at offset 0 so we can do a memcpy into it */
+ __u8 data[10];
+ unsigned int hid;
+ int retval;
+ size_t size;
+ enum hid_report_type type;
+ __u8 request_type;
+};
+
+SEC("syscall")
+int hid_user_raw_request(struct hid_hw_request_syscall_args *args)
+{
+ struct hid_bpf_ctx *ctx;
+ const size_t size = args->size;
+ int i, ret = 0;
+
+ if (size > sizeof(args->data))
+ return -7; /* -E2BIG */
+
+ ctx = hid_bpf_allocate_context(args->hid);
+ if (!ctx)
+ return -1; /* EPERM check */
+
+ ret = hid_bpf_hw_request(ctx,
+ args->data,
+ size,
+ args->type,
+ args->request_type);
+ args->retval = ret;
+
+ hid_bpf_release_context(ctx);
+
+ return 0;
+}
--
2.36.1

2022-07-12 15:19:49

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 22/23] HID: bpf: add Surface Dial example

Add a more complete HID-BPF example.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

new in v6
---
samples/bpf/.gitignore | 1 +
samples/bpf/Makefile | 6 +-
samples/bpf/hid_surface_dial.bpf.c | 161 +++++++++++++++++++++
samples/bpf/hid_surface_dial.c | 216 +++++++++++++++++++++++++++++
4 files changed, 383 insertions(+), 1 deletion(-)
create mode 100644 samples/bpf/hid_surface_dial.bpf.c
create mode 100644 samples/bpf/hid_surface_dial.c

diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
index 65440bd618b2..6a1079d3d064 100644
--- a/samples/bpf/.gitignore
+++ b/samples/bpf/.gitignore
@@ -3,6 +3,7 @@ cpustat
fds_example
hbm
hid_mouse
+hid_surface_dial
ibumad
lathist
lwt_len_hist
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index e67c1a0fed1c..d1e40aff0e55 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -58,6 +58,7 @@ tprogs-y += xdp_redirect
tprogs-y += xdp_monitor

tprogs-y += hid_mouse
+tprogs-y += hid_surface_dial

# Libbpf dependencies
LIBBPF_SRC = $(TOOLS_PATH)/lib/bpf
@@ -122,6 +123,7 @@ xdp_monitor-objs := xdp_monitor_user.o $(XDP_SAMPLE)
xdp_router_ipv4-objs := xdp_router_ipv4_user.o $(XDP_SAMPLE)

hid_mouse-objs := hid_mouse.o
+hid_surface_dial-objs := hid_surface_dial.o

# Tell kbuild to always build the programs
always-y := $(tprogs-y)
@@ -345,6 +347,7 @@ $(obj)/hbm.o: $(src)/hbm.h
$(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h

$(obj)/hid_mouse.o: $(obj)/hid_mouse.skel.h
+$(obj)/hid_surface_dial.o: $(obj)/hid_surface_dial.skel.h

# Override includes for xdp_sample_user.o because $(srctree)/usr/include in
# TPROGS_CFLAGS causes conflicts
@@ -431,9 +434,10 @@ $(BPF_SKELS_LINKED): $(BPF_OBJS_LINKED) $(BPFTOOL)
$(Q)$(BPFTOOL) gen skeleton $(@:.skel.h=.lbpf.o) name $(notdir $(@:.skel.h=)) > $@

# Generate BPF skeletons for non XDP progs
-OTHER_BPF_SKELS := hid_mouse.skel.h
+OTHER_BPF_SKELS := hid_mouse.skel.h hid_surface_dial.skel.h

hid_mouse.skel.h-deps := hid_mouse.bpf.o
+hid_surface_dial.skel.h-deps := hid_surface_dial.bpf.o

OTHER_BPF_SRCS_LINKED := $(patsubst %.skel.h,%.bpf.c, $(OTHER_BPF_SKELS))
OTHER_BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(OTHER_BPF_SRCS_LINKED))
diff --git a/samples/bpf/hid_surface_dial.bpf.c b/samples/bpf/hid_surface_dial.bpf.c
new file mode 100644
index 000000000000..16c821d3decf
--- /dev/null
+++ b/samples/bpf/hid_surface_dial.bpf.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2022 Benjamin Tissoires
+ */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+#define HID_UP_BUTTON 0x0009
+#define HID_GD_WHEEL 0x0038
+
+/* following are kfuncs exported by HID for HID-BPF */
+extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
+ unsigned int offset,
+ const size_t __sz) __ksym;
+extern int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, u32 flags) __ksym;
+extern struct hid_bpf_ctx *hid_bpf_allocate_context(unsigned int hid_id) __ksym;
+extern void hid_bpf_release_context(struct hid_bpf_ctx *ctx) __ksym;
+extern int hid_bpf_hw_request(struct hid_bpf_ctx *ctx,
+ __u8 *data,
+ size_t buf__sz,
+ enum hid_report_type type,
+ enum hid_class_request reqtype) __ksym;
+
+struct attach_prog_args {
+ int prog_fd;
+ unsigned int hid;
+ int retval;
+};
+
+SEC("syscall")
+int attach_prog(struct attach_prog_args *ctx)
+{
+ ctx->retval = hid_bpf_attach_prog(ctx->hid,
+ ctx->prog_fd,
+ 0);
+ return 0;
+}
+
+SEC("fmod_ret/hid_bpf_device_event")
+int BPF_PROG(hid_event, struct hid_bpf_ctx *hctx)
+{
+ __u8 *data = hid_bpf_get_data(hctx, 0 /* offset */, 9 /* size */);
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ /* Touch */
+ data[1] &= 0xfd;
+
+ /* X */
+ data[4] = 0;
+ data[5] = 0;
+
+ /* Y */
+ data[6] = 0;
+ data[7] = 0;
+
+ return 0;
+}
+
+/* 72 == 360 / 5 -> 1 report every 5 degrees */
+int resolution = 72;
+int physical = 5;
+
+struct haptic_syscall_args {
+ unsigned int hid;
+ int retval;
+};
+
+static __u8 haptic_data[8];
+
+SEC("syscall")
+int set_haptic(struct haptic_syscall_args *args)
+{
+ struct hid_bpf_ctx *ctx;
+ const size_t size = sizeof(haptic_data);
+ u16 *res;
+ int ret;
+
+ if (size > sizeof(haptic_data))
+ return -7; /* -E2BIG */
+
+ ctx = hid_bpf_allocate_context(args->hid);
+ if (!ctx)
+ return -1; /* EPERM check */
+
+ haptic_data[0] = 1; /* report ID */
+
+ ret = hid_bpf_hw_request(ctx, haptic_data, size, HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+
+ bpf_printk("probed/remove event ret value: %d", ret);
+ bpf_printk("buf: %02x %02x %02x",
+ haptic_data[0],
+ haptic_data[1],
+ haptic_data[2]);
+ bpf_printk(" %02x %02x %02x",
+ haptic_data[3],
+ haptic_data[4],
+ haptic_data[5]);
+ bpf_printk(" %02x %02x",
+ haptic_data[6],
+ haptic_data[7]);
+
+ /* whenever resolution multiplier is not 3600, we have the fixed report descriptor */
+ res = (u16 *)&haptic_data[1];
+ if (*res != 3600) {
+// haptic_data[1] = 72; /* resolution multiplier */
+// haptic_data[2] = 0; /* resolution multiplier */
+// haptic_data[3] = 0; /* Repeat Count */
+ haptic_data[4] = 3; /* haptic Auto Trigger */
+// haptic_data[5] = 5; /* Waveform Cutoff Time */
+// haptic_data[6] = 80; /* Retrigger Period */
+// haptic_data[7] = 0; /* Retrigger Period */
+ } else {
+ haptic_data[4] = 0;
+ }
+
+ ret = hid_bpf_hw_request(ctx, haptic_data, size, HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+
+ bpf_printk("set haptic ret value: %d -> %d", ret, haptic_data[4]);
+
+ args->retval = ret;
+
+ hid_bpf_release_context(ctx);
+
+ return 0;
+}
+
+/* Convert REL_DIAL into REL_WHEEL */
+SEC("fmod_ret/hid_bpf_rdesc_fixup")
+int BPF_PROG(hid_rdesc_fixup, struct hid_bpf_ctx *hctx)
+{
+ __u8 *data = hid_bpf_get_data(hctx, 0 /* offset */, 4096 /* size */);
+ __u16 *res, *phys;
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ /* Convert TOUCH into a button */
+ data[31] = HID_UP_BUTTON;
+ data[33] = 2;
+
+ /* Convert REL_DIAL into REL_WHEEL */
+ data[45] = HID_GD_WHEEL;
+
+ /* Change Resolution Multiplier */
+ phys = (__u16 *)&data[61];
+ *phys = physical;
+ res = (__u16 *)&data[66];
+ *res = resolution;
+
+ /* Convert X,Y from Abs to Rel */
+ data[88] = 0x06;
+ data[98] = 0x06;
+
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+u32 _version SEC("version") = 1;
diff --git a/samples/bpf/hid_surface_dial.c b/samples/bpf/hid_surface_dial.c
new file mode 100644
index 000000000000..b901c578afac
--- /dev/null
+++ b/samples/bpf/hid_surface_dial.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2022 Benjamin Tissoires
+ */
+
+
+/* not sure why but this doesn't get preoperly imported */
+#define __must_check
+
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <libgen.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <unistd.h>
+
+#include <linux/bpf.h>
+#include <linux/errno.h>
+
+#include "bpf_util.h"
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "hid_surface_dial.skel.h"
+
+static bool running = true;
+
+struct attach_prog_args {
+ int prog_fd;
+ unsigned int hid;
+ int retval;
+};
+
+struct haptic_syscall_args {
+ unsigned int hid;
+ int retval;
+};
+
+static void int_exit(int sig)
+{
+ running = false;
+ exit(0);
+}
+
+static void usage(const char *prog)
+{
+ fprintf(stderr,
+ "%s: %s [OPTIONS] /sys/bus/hid/devices/0BUS:0VID:0PID:00ID\n\n"
+ " OPTIONS:\n"
+ " -r N\t set the given resolution to the device (number of ticks per 360°)\n\n",
+ __func__, prog);
+}
+
+static int get_hid_id(const char *path)
+{
+ const char *str_id, *dir;
+ char uevent[1024];
+ int fd;
+
+ memset(uevent, 0, sizeof(uevent));
+ snprintf(uevent, sizeof(uevent) - 1, "%s/uevent", path);
+
+ fd = open(uevent, O_RDONLY | O_NONBLOCK);
+ if (fd < 0)
+ return -ENOENT;
+
+ close(fd);
+
+ dir = basename((char *)path);
+
+ str_id = dir + sizeof("0003:0001:0A37.");
+ return (int)strtol(str_id, NULL, 16);
+}
+
+static int attach_prog(struct hid_surface_dial_lskel *skel, struct bpf_program *prog, int hid_id)
+{
+ struct attach_prog_args args = {
+ .hid = hid_id,
+ .retval = -1,
+ };
+ int attach_fd, err;
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattr,
+ .ctx_in = &args,
+ .ctx_size_in = sizeof(args),
+ );
+
+ attach_fd = bpf_program__fd(skel->progs.attach_prog);
+ if (attach_fd < 0) {
+ fprintf(stderr, "can't locate attach prog: %m\n");
+ return 1;
+ }
+
+ args.prog_fd = bpf_program__fd(prog);
+ err = bpf_prog_test_run_opts(attach_fd, &tattr);
+ if (err) {
+ fprintf(stderr, "can't attach prog to hid device %d: %m (err: %d)\n",
+ hid_id, err);
+ return 1;
+ }
+ return 0;
+}
+
+static int set_haptic(struct hid_surface_dial_lskel *skel, int hid_id)
+{
+ struct haptic_syscall_args args = {
+ .hid = hid_id,
+ .retval = -1,
+ };
+ int haptic_fd, err;
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattr,
+ .ctx_in = &args,
+ .ctx_size_in = sizeof(args),
+ );
+
+ haptic_fd = bpf_program__fd(skel->progs.set_haptic);
+ if (haptic_fd < 0) {
+ fprintf(stderr, "can't locate haptic prog: %m\n");
+ return 1;
+ }
+
+ err = bpf_prog_test_run_opts(haptic_fd, &tattr);
+ if (err) {
+ fprintf(stderr, "can't set haptic configuration to hid device %d: %m (err: %d)\n",
+ hid_id, err);
+ return 1;
+ }
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ struct hid_surface_dial_lskel *skel;
+ struct bpf_program *prog;
+ const char *optstr = "r:";
+ const char *sysfs_path;
+ int opt, hid_id, resolution = 72;
+
+ while ((opt = getopt(argc, argv, optstr)) != -1) {
+ switch (opt) {
+ case 'r':
+ {
+ char *endp = NULL;
+ long l = -1;
+
+ if (optarg) {
+ l = strtol(optarg, &endp, 10);
+ if (endp && *endp)
+ l = -1;
+ }
+
+ if (l < 0) {
+ fprintf(stderr,
+ "invalid r option %s - expecting a number\n",
+ optarg ? optarg : "");
+ exit(EXIT_FAILURE);
+ };
+
+ resolution = (int) l;
+ break;
+ }
+ 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;
+ }
+
+ skel = hid_surface_dial_lskel__open_and_load();
+ if (!skel) {
+ fprintf(stderr, "%s %s:%d", __func__, __FILE__, __LINE__);
+ return -1;
+ }
+
+ hid_id = get_hid_id(sysfs_path);
+ if (hid_id < 0) {
+ fprintf(stderr, "can not open HID device: %m\n");
+ return 1;
+ }
+
+ skel->data->resolution = resolution;
+ skel->data->physical = (int)(resolution / 72);
+
+ bpf_object__for_each_program(prog, *skel->skeleton->obj) {
+ /* ignore syscalls */
+ if (bpf_program__get_type(prog) != BPF_PROG_TYPE_TRACING)
+ continue;
+
+ attach_prog(skel, prog, hid_id);
+ }
+
+ signal(SIGINT, int_exit);
+ signal(SIGTERM, int_exit);
+
+ set_haptic(skel, hid_id);
+
+ while (running)
+ ;
+
+ hid_surface_dial_lskel__destroy(skel);
+
+ return 0;
+}
--
2.36.1

2022-07-12 15:21:20

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 21/23] samples/bpf: add new hid_mouse example

Everything should be available in the selftest part of the tree, but
providing an example without uhid and hidraw will be more easy to
follow for users.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

changes in v6:
- clean up code by removing old comments

changes in v5:
- bring back same features than v3, with the new API

changes in v4:
- dropped the not-yet-implemented rdesc_fixup
- use the new API

changes in v3:
- use the new hid_get_data API
- add a comment for the report descriptor fixup to explain what is done

changes in v2:
- split the series by bpf/libbpf/hid/selftests and samples
---
samples/bpf/.gitignore | 1 +
samples/bpf/Makefile | 23 ++++++
samples/bpf/hid_mouse.bpf.c | 134 ++++++++++++++++++++++++++++++++
samples/bpf/hid_mouse.c | 150 ++++++++++++++++++++++++++++++++++++
4 files changed, 308 insertions(+)
create mode 100644 samples/bpf/hid_mouse.bpf.c
create mode 100644 samples/bpf/hid_mouse.c

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 5002a5b9a7da..e67c1a0fed1c 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -57,6 +57,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
@@ -119,6 +121,8 @@ xdp_redirect-objs := xdp_redirect_user.o $(XDP_SAMPLE)
xdp_monitor-objs := xdp_monitor_user.o $(XDP_SAMPLE)
xdp_router_ipv4-objs := xdp_router_ipv4_user.o $(XDP_SAMPLE)

+hid_mouse-objs := hid_mouse.o
+
# Tell kbuild to always build the programs
always-y := $(tprogs-y)
always-y += sockex1_kern.o
@@ -340,6 +344,8 @@ $(obj)/hbm_out_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
$(obj)/hbm.o: $(src)/hbm.h
$(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h

+$(obj)/hid_mouse.o: $(obj)/hid_mouse.skel.h
+
# Override includes for xdp_sample_user.o because $(srctree)/usr/include in
# TPROGS_CFLAGS causes conflicts
XDP_SAMPLE_CFLAGS += -Wall -O2 \
@@ -424,6 +430,23 @@ $(BPF_SKELS_LINKED): $(BPF_OBJS_LINKED) $(BPFTOOL)
@echo " BPF GEN-SKEL" $(@:.skel.h=)
$(Q)$(BPFTOOL) gen skeleton $(@:.skel.h=.lbpf.o) name $(notdir $(@:.skel.h=)) > $@

+# Generate BPF skeletons for non XDP progs
+OTHER_BPF_SKELS := hid_mouse.skel.h
+
+hid_mouse.skel.h-deps := hid_mouse.bpf.o
+
+OTHER_BPF_SRCS_LINKED := $(patsubst %.skel.h,%.bpf.c, $(OTHER_BPF_SKELS))
+OTHER_BPF_OBJS_LINKED := $(patsubst %.bpf.c,$(obj)/%.bpf.o, $(OTHER_BPF_SRCS_LINKED))
+OTHER_BPF_SKELS_LINKED := $(addprefix $(obj)/,$(OTHER_BPF_SKELS))
+
+$(OTHER_BPF_SKELS_LINKED): $(OTHER_BPF_OBJS_LINKED) $(BPFTOOL)
+ @echo " BPF GEN-OBJ " $(@:.skel.h=)
+ $(Q)$(BPFTOOL) gen object $(@:.skel.h=.lbpf.o) $(addprefix $(obj)/,$($(@F)-deps))
+ @echo " BPF GEN-SKEL" $(@:.skel.h=)
+ $(Q)$(BPFTOOL) gen skeleton $(@:.skel.h=.lbpf.o) name $(notdir $(@:.skel.h=_lskel)) > $@
+# $(call msg,GEN-SKEL,$@)
+# $(Q)$(BPFTOOL) gen skeleton $< > $@
+
# asm/sysreg.h - inline assembly used by it is incompatible with llvm.
# But, there is no easy way to fix it, so just exclude it since it is
# useless for BPF samples.
diff --git a/samples/bpf/hid_mouse.bpf.c b/samples/bpf/hid_mouse.bpf.c
new file mode 100644
index 000000000000..0113e603f7a7
--- /dev/null
+++ b/samples/bpf/hid_mouse.bpf.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+/* following are kfuncs exported by HID for HID-BPF */
+extern int hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, u32 flags) __ksym;
+extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
+ unsigned int offset,
+ const size_t __sz) __ksym;
+extern void hid_bpf_data_release(__u8 *data) __ksym;
+extern int hid_bpf_hw_request(struct hid_bpf_ctx *ctx) __ksym;
+
+struct attach_prog_args {
+ int prog_fd;
+ unsigned int hid;
+ int retval;
+};
+
+SEC("syscall")
+int attach_prog(struct attach_prog_args *ctx)
+{
+ ctx->retval = hid_bpf_attach_prog(ctx->hid,
+ ctx->prog_fd,
+ 0);
+ return 0;
+}
+
+SEC("fmod_ret/hid_bpf_device_event")
+int BPF_PROG(hid_y_event, struct hid_bpf_ctx *hctx)
+{
+ s16 y;
+ __u8 *data = hid_bpf_get_data(hctx, 0 /* offset */, 9 /* size */);
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ bpf_printk("event: size: %d", hctx->size);
+ bpf_printk("incoming event: %02x %02x %02x",
+ data[0],
+ data[1],
+ data[2]);
+ bpf_printk(" %02x %02x %02x",
+ data[3],
+ data[4],
+ data[5]);
+ bpf_printk(" %02x %02x %02x",
+ data[6],
+ data[7],
+ data[8]);
+
+ y = data[3] | (data[4] << 8);
+
+ y = -y;
+
+ data[3] = y & 0xFF;
+ data[4] = (y >> 8) & 0xFF;
+
+ bpf_printk("modified event: %02x %02x %02x",
+ data[0],
+ data[1],
+ data[2]);
+ bpf_printk(" %02x %02x %02x",
+ data[3],
+ data[4],
+ data[5]);
+ bpf_printk(" %02x %02x %02x",
+ data[6],
+ data[7],
+ data[8]);
+
+ return 0;
+}
+
+SEC("fmod_ret/hid_bpf_device_event")
+int BPF_PROG(hid_x_event, struct hid_bpf_ctx *hctx)
+{
+ s16 x;
+ __u8 *data = hid_bpf_get_data(hctx, 0 /* offset */, 9 /* size */);
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ x = data[1] | (data[2] << 8);
+
+ x = -x;
+
+ data[1] = x & 0xFF;
+ data[2] = (x >> 8) & 0xFF;
+ return 0;
+}
+
+SEC("fmod_ret/hid_bpf_rdesc_fixup")
+int BPF_PROG(hid_rdesc_fixup, struct hid_bpf_ctx *hctx)
+{
+ __u8 *data = hid_bpf_get_data(hctx, 0 /* offset */, 4096 /* size */);
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ bpf_printk("rdesc: %02x %02x %02x",
+ data[0],
+ data[1],
+ data[2]);
+ bpf_printk(" %02x %02x %02x",
+ data[3],
+ data[4],
+ data[5]);
+ bpf_printk(" %02x %02x %02x ...",
+ data[6],
+ data[7],
+ data[8]);
+
+ /*
+ * The original report descriptor contains:
+ *
+ * 0x05, 0x01, // Usage Page (Generic Desktop) 30
+ * 0x16, 0x01, 0x80, // Logical Minimum (-32767) 32
+ * 0x26, 0xff, 0x7f, // Logical Maximum (32767) 35
+ * 0x09, 0x30, // Usage (X) 38
+ * 0x09, 0x31, // Usage (Y) 40
+ *
+ * So byte 39 contains Usage X and byte 41 Usage Y.
+ *
+ * We simply swap the axes here.
+ */
+ data[39] = 0x31;
+ data[41] = 0x30;
+
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/hid_mouse.c b/samples/bpf/hid_mouse.c
new file mode 100644
index 000000000000..f6e5f09026eb
--- /dev/null
+++ b/samples/bpf/hid_mouse.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2022 Benjamin Tissoires
+ */
+
+/* not sure why but this doesn't get preoperly imported */
+#define __must_check
+
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <libgen.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <unistd.h>
+
+#include <linux/bpf.h>
+#include <linux/errno.h>
+
+#include "bpf_util.h"
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "hid_mouse.skel.h"
+
+static bool running = true;
+
+struct attach_prog_args {
+ int prog_fd;
+ unsigned int hid;
+ int retval;
+};
+
+static void int_exit(int sig)
+{
+ running = false;
+ exit(0);
+}
+
+static void usage(const char *prog)
+{
+ fprintf(stderr,
+ "%s: %s /sys/bus/hid/devices/0BUS:0VID:0PID:00ID\n\n",
+ __func__, prog);
+}
+
+static int get_hid_id(const char *path)
+{
+ const char *str_id, *dir;
+ char uevent[1024];
+ int fd;
+
+ memset(uevent, 0, sizeof(uevent));
+ snprintf(uevent, sizeof(uevent) - 1, "%s/uevent", path);
+
+ fd = open(uevent, O_RDONLY | O_NONBLOCK);
+ if (fd < 0)
+ return -ENOENT;
+
+ close(fd);
+
+ dir = basename((char *)path);
+
+ str_id = dir + sizeof("0003:0001:0A37.");
+ return (int)strtol(str_id, NULL, 16);
+}
+
+int main(int argc, char **argv)
+{
+ struct hid_mouse_lskel *skel;
+ struct bpf_program *prog;
+ int err;
+ const char *optstr = "";
+ const char *sysfs_path;
+ int opt, hid_id, attach_fd;
+ struct attach_prog_args args = {
+ .retval = -1,
+ };
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattr,
+ .ctx_in = &args,
+ .ctx_size_in = sizeof(args),
+ );
+
+ 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;
+ }
+
+ skel = hid_mouse_lskel__open_and_load();
+ if (!skel) {
+ fprintf(stderr, "%s %s:%d", __func__, __FILE__, __LINE__);
+ return -1;
+ }
+
+ hid_id = get_hid_id(sysfs_path);
+
+ if (hid_id < 0) {
+ fprintf(stderr, "can not open HID device: %m\n");
+ return 1;
+ }
+ args.hid = hid_id;
+
+ attach_fd = bpf_program__fd(skel->progs.attach_prog);
+ if (attach_fd < 0) {
+ fprintf(stderr, "can't locate attach prog: %m\n");
+ return 1;
+ }
+
+ bpf_object__for_each_program(prog, *skel->skeleton->obj) {
+ /* ignore syscalls */
+ if (bpf_program__get_type(prog) != BPF_PROG_TYPE_TRACING)
+ continue;
+
+ args.retval = -1;
+ args.prog_fd = bpf_program__fd(prog);
+ err = bpf_prog_test_run_opts(attach_fd, &tattr);
+ if (err) {
+ fprintf(stderr, "can't attach prog to hid device %d: %m (err: %d)\n",
+ hid_id, err);
+ return 1;
+ }
+ }
+
+ signal(SIGINT, int_exit);
+ signal(SIGTERM, int_exit);
+
+ while (running)
+ ;
+
+ hid_mouse_lskel__destroy(skel);
+
+ return 0;
+}
--
2.36.1

2022-07-12 15:22:10

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 11/23] HID: convert defines of HID class requests into a proper enum

This allows to export the type in BTF and so in the automatically
generated vmlinux.h. It will also add some static checks on the users
when we change the ll driver API (see not below).

Note that we need to also do change in the ll_driver API, but given
that this will have a wider impact outside of this tree, we leave this
as a TODO for the future.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

new in v6
---
drivers/hid/hid-core.c | 6 +++---
include/linux/hid.h | 9 +++++----
include/uapi/linux/hid.h | 14 ++++++++------
3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 1c9c15a12f24..b2257f3a93d1 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1921,7 +1921,7 @@ static struct hid_report *hid_get_report(struct hid_report_enum *report_enum,
* DO NOT USE in hid drivers directly, but through hid_hw_request instead.
*/
int __hid_request(struct hid_device *hid, struct hid_report *report,
- int reqtype)
+ enum hid_class_request reqtype)
{
char *buf;
int ret;
@@ -2353,7 +2353,7 @@ EXPORT_SYMBOL_GPL(hid_hw_close);
* @reqtype: hid request type
*/
void hid_hw_request(struct hid_device *hdev,
- struct hid_report *report, int reqtype)
+ struct hid_report *report, enum hid_class_request reqtype)
{
if (hdev->ll_driver->request)
return hdev->ll_driver->request(hdev, report, reqtype);
@@ -2378,7 +2378,7 @@ EXPORT_SYMBOL_GPL(hid_hw_request);
*/
int hid_hw_raw_request(struct hid_device *hdev,
unsigned char reportnum, __u8 *buf,
- size_t len, enum hid_report_type rtype, int reqtype)
+ size_t len, enum hid_report_type rtype, enum hid_class_request reqtype)
{
if (len < 1 || len > HID_MAX_BUFFER_SIZE || !buf)
return -EINVAL;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index b1a33dbbc78e..8677ae38599e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -923,7 +923,7 @@ struct hid_field *hidinput_get_led_field(struct hid_device *hid);
unsigned int hidinput_count_leds(struct hid_device *hid);
__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
void hid_output_report(struct hid_report *report, __u8 *data);
-int __hid_request(struct hid_device *hid, struct hid_report *rep, int reqtype);
+int __hid_request(struct hid_device *hid, struct hid_report *rep, enum hid_class_request reqtype);
u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags);
struct hid_device *hid_allocate_device(void);
struct hid_report *hid_register_report(struct hid_device *device,
@@ -1100,10 +1100,11 @@ void hid_hw_stop(struct hid_device *hdev);
int __must_check hid_hw_open(struct hid_device *hdev);
void hid_hw_close(struct hid_device *hdev);
void hid_hw_request(struct hid_device *hdev,
- struct hid_report *report, int reqtype);
+ struct hid_report *report, enum hid_class_request reqtype);
int hid_hw_raw_request(struct hid_device *hdev,
unsigned char reportnum, __u8 *buf,
- size_t len, enum hid_report_type rtype, int reqtype);
+ size_t len, enum hid_report_type rtype,
+ enum hid_class_request reqtype);
int hid_hw_output_report(struct hid_device *hdev, __u8 *buf, size_t len);

/**
@@ -1131,7 +1132,7 @@ static inline int hid_hw_power(struct hid_device *hdev, int level)
* @reqtype: hid request type
*/
static inline int hid_hw_idle(struct hid_device *hdev, int report, int idle,
- int reqtype)
+ enum hid_class_request reqtype)
{
if (hdev->ll_driver->idle)
return hdev->ll_driver->idle(hdev, report, idle, reqtype);
diff --git a/include/uapi/linux/hid.h b/include/uapi/linux/hid.h
index b25b0bacaff2..a4dcb34386e3 100644
--- a/include/uapi/linux/hid.h
+++ b/include/uapi/linux/hid.h
@@ -58,12 +58,14 @@ enum hid_report_type {
* HID class requests
*/

-#define HID_REQ_GET_REPORT 0x01
-#define HID_REQ_GET_IDLE 0x02
-#define HID_REQ_GET_PROTOCOL 0x03
-#define HID_REQ_SET_REPORT 0x09
-#define HID_REQ_SET_IDLE 0x0A
-#define HID_REQ_SET_PROTOCOL 0x0B
+enum hid_class_request {
+ HID_REQ_GET_REPORT = 0x01,
+ HID_REQ_GET_IDLE = 0x02,
+ HID_REQ_GET_PROTOCOL = 0x03,
+ HID_REQ_SET_REPORT = 0x09,
+ HID_REQ_SET_IDLE = 0x0A,
+ HID_REQ_SET_PROTOCOL = 0x0B,
+};

/*
* HID class descriptor types
--
2.36.1

2022-07-12 15:23:45

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 18/23] HID: bpf: allow to change the report descriptor

Add a new tracepoint hid_bpf_rdesc_fixup() so we can trigger a
report descriptor fixup in the bpf world.

Whenever the program gets attached/detached, the device is reconnected
meaning that userspace will see it disappearing and reappearing with
the new report descriptor.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

changes in v6:
- use BTF_ID to get the btf_id of hid_bpf_rdesc_fixup

changes in v5:
- adapted for new API

not in v4

changes in v3:
- ensure the ctx.size is properly bounded by allocated size
- s/link_attached/post_link_attach/
- removed the switch statement with only one case

changes in v2:
- split the series by bpf/libbpf/hid/selftests and samples
---
drivers/hid/bpf/hid_bpf_dispatch.c | 77 ++++++++++++++++++++++++++++-
drivers/hid/bpf/hid_bpf_dispatch.h | 1 +
drivers/hid/bpf/hid_bpf_jmp_table.c | 7 +++
drivers/hid/hid-core.c | 3 +-
include/linux/hid_bpf.h | 8 +++
5 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 8348f5ae17f8..bcaeb65ed2a8 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -85,6 +85,63 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
}
EXPORT_SYMBOL_GPL(dispatch_hid_bpf_device_event);

+/**
+ * hid_bpf_rdesc_fixup - Called when the probe function parses the report
+ * descriptor of the HID device
+ *
+ * @ctx: The HID-BPF context
+ *
+ * @return 0 on success and keep processing; a positive value to change the
+ * incoming size buffer; a negative error code to interrupt the processing
+ * of this event
+ *
+ * Declare an %fmod_ret tracing bpf program to this function and attach this
+ * program through hid_bpf_attach_prog() to have this helper called before any
+ * parsing of the report descriptor by HID.
+ */
+/* never used by the kernel but declared so we can load and attach a tracepoint */
+__weak noinline int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx)
+{
+ return 0;
+}
+ALLOW_ERROR_INJECTION(hid_bpf_rdesc_fixup, ERRNO);
+
+u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size)
+{
+ int ret;
+ struct hid_bpf_ctx_kern ctx_kern = {
+ .ctx = {
+ .hid = hdev,
+ .size = *size,
+ .allocated_size = HID_MAX_DESCRIPTOR_SIZE,
+ },
+ };
+
+ ctx_kern.data = kmemdup(rdesc, ctx_kern.ctx.allocated_size, GFP_KERNEL);
+ if (!ctx_kern.data)
+ goto ignore_bpf;
+
+ ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_RDESC_FIXUP, &ctx_kern);
+ if (ret < 0)
+ goto ignore_bpf;
+
+ if (ret) {
+ if (ret > ctx_kern.ctx.allocated_size)
+ goto ignore_bpf;
+
+ *size = ret;
+ }
+
+ rdesc = krealloc(ctx_kern.data, *size, GFP_KERNEL);
+
+ return rdesc;
+
+ ignore_bpf:
+ kfree(ctx_kern.data);
+ return kmemdup(rdesc, *size, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
+
/**
* hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx
*
@@ -188,6 +245,14 @@ static int hid_bpf_allocate_event_data(struct hid_device *hdev)
return __hid_bpf_allocate_data(hdev, &hdev->bpf.device_data, &hdev->bpf.allocated_data);
}

+int hid_bpf_reconnect(struct hid_device *hdev)
+{
+ if (!test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status))
+ return device_reprobe(&hdev->dev);
+
+ return 0;
+}
+
/**
* hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
*
@@ -229,7 +294,17 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
return err;
}

- return __hid_bpf_attach_prog(hdev, prog_type, prog_fd, flags);
+ err = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, flags);
+ if (err)
+ return err;
+
+ if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
+ err = hid_bpf_reconnect(hdev);
+ if (err)
+ return err;
+ }
+
+ return 0;
}

/**
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.h b/drivers/hid/bpf/hid_bpf_dispatch.h
index 98c378e18b2b..1d1d5bcccbd7 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.h
+++ b/drivers/hid/bpf/hid_bpf_dispatch.h
@@ -18,6 +18,7 @@ int __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_t
void __hid_bpf_destroy_device(struct hid_device *hdev);
int hid_bpf_prog_run(struct hid_device *hdev, enum hid_bpf_prog_type type,
struct hid_bpf_ctx_kern *ctx_kern);
+int hid_bpf_reconnect(struct hid_device *hdev);

struct bpf_prog;

diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index 0f20deab81ff..3a5ab70d1a95 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -59,12 +59,15 @@ static DECLARE_WORK(release_work, hid_bpf_release_progs);

BTF_ID_LIST(hid_bpf_btf_ids)
BTF_ID(func, hid_bpf_device_event) /* HID_BPF_PROG_TYPE_DEVICE_EVENT */
+BTF_ID(func, hid_bpf_rdesc_fixup) /* HID_BPF_PROG_TYPE_RDESC_FIXUP */

static int hid_bpf_max_programs(enum hid_bpf_prog_type type)
{
switch (type) {
case HID_BPF_PROG_TYPE_DEVICE_EVENT:
return HID_BPF_MAX_PROGS_PER_DEV;
+ case HID_BPF_PROG_TYPE_RDESC_FIXUP:
+ return 1;
default:
return -EINVAL;
}
@@ -234,6 +237,10 @@ static void hid_bpf_release_progs(struct work_struct *work)
if (next->hdev == hdev && next->type == type)
next->hdev = NULL;
}
+
+ /* if type was rdesc fixup, reconnect device */
+ if (type == HID_BPF_PROG_TYPE_RDESC_FIXUP)
+ hid_bpf_reconnect(hdev);
}
}

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 356d3822f17d..42aed754123c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1218,7 +1218,8 @@ int hid_open_report(struct hid_device *device)
return -ENODEV;
size = device->dev_rsize;

- buf = kmemdup(start, size, GFP_KERNEL);
+ /* call_hid_bpf_rdesc_fixup() ensures we work on a copy of rdesc */
+ buf = call_hid_bpf_rdesc_fixup(device, start, &size);
if (buf == NULL)
return -ENOMEM;

diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index ade0def154b6..4f5eb27a188b 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -59,6 +59,7 @@ struct hid_bpf_ctx {

/* Following functions are tracepoints that BPF programs can attach to */
int hid_bpf_device_event(struct hid_bpf_ctx *ctx);
+int hid_bpf_rdesc_fixup(struct hid_bpf_ctx *ctx);

/* Following functions are kfunc that we export to BPF programs */
/* available everywhere in HID-BPF */
@@ -82,6 +83,7 @@ void hid_bpf_release_context(struct hid_bpf_ctx *ctx);
enum hid_bpf_prog_type {
HID_BPF_PROG_TYPE_UNDEF = -1,
HID_BPF_PROG_TYPE_DEVICE_EVENT, /* an event is emitted from the device */
+ HID_BPF_PROG_TYPE_RDESC_FIXUP,
HID_BPF_PROG_TYPE_MAX,
};

@@ -125,6 +127,7 @@ int hid_bpf_connect_device(struct hid_device *hdev);
void hid_bpf_disconnect_device(struct hid_device *hdev);
void hid_bpf_destroy_device(struct hid_device *hid);
void hid_bpf_device_init(struct hid_device *hid);
+u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size);
#else /* CONFIG_BPF */
static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, int type, u8 *data,
u32 *size, int interrupt) { return 0; }
@@ -132,6 +135,11 @@ static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; }
static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {}
static inline void hid_bpf_destroy_device(struct hid_device *hid) {}
static inline void hid_bpf_device_init(struct hid_device *hid) {}
+static inline u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size)
+{
+ return kmemdup(rdesc, *size, GFP_KERNEL);
+}
+
#endif /* CONFIG_BPF */

#endif /* __HID_BPF_H */
--
2.36.1

2022-07-12 15:35:44

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 04/23] selftests/bpf: add test for accessing ctx from syscall program type

We need to also export the kfunc set to the syscall program type,
and then add a couple of eBPF programs that are testing those calls.

The first one checks for valid access, and the second one is OK
from a static analysis point of view but fails at run time because
we are trying to access outside of the allocated memory.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

new in v6
---
net/bpf/test_run.c | 1 +
.../selftests/bpf/prog_tests/kfunc_call.c | 20 ++++++++++++++
.../selftests/bpf/progs/kfunc_call_test.c | 27 +++++++++++++++++++
3 files changed, 48 insertions(+)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 2ca96acbc50a..9da2a42811e8 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1646,6 +1646,7 @@ static int __init bpf_prog_test_run_init(void)
int ret;

ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_prog_test_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_prog_test_kfunc_set);
return ret ?: register_btf_id_dtor_kfuncs(bpf_prog_test_dtor_kfunc,
ARRAY_SIZE(bpf_prog_test_dtor_kfunc),
THIS_MODULE);
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
index c00eb974eb85..22547aafdd60 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
@@ -6,10 +6,22 @@
#include "kfunc_call_test_subprog.skel.h"
#include "kfunc_call_test_subprog.lskel.h"

+struct syscall_test_args {
+ __u8 data[16];
+ size_t size;
+};
+
static void test_main(void)
{
struct kfunc_call_test_lskel *skel;
int prog_fd, err;
+ struct syscall_test_args args = {
+ .size = 10,
+ };
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, syscall_topts,
+ .ctx_in = &args,
+ .ctx_size_in = sizeof(args),
+ );
LIBBPF_OPTS(bpf_test_run_opts, topts,
.data_in = &pkt_v4,
.data_size_in = sizeof(pkt_v4),
@@ -35,6 +47,14 @@ static void test_main(void)
ASSERT_OK(err, "bpf_prog_test_run(test_ref_btf_id)");
ASSERT_EQ(topts.retval, 0, "test_ref_btf_id-retval");

+ prog_fd = skel->progs.kfunc_syscall_test.prog_fd;
+ err = bpf_prog_test_run_opts(prog_fd, &syscall_topts);
+ ASSERT_OK(err, "bpf_prog_test_run(syscall_test)");
+
+ prog_fd = skel->progs.kfunc_syscall_test_fail.prog_fd;
+ err = bpf_prog_test_run_opts(prog_fd, &syscall_topts);
+ ASSERT_ERR(err, "bpf_prog_test_run(syscall_test_fail)");
+
kfunc_call_test_lskel__destroy(skel);
}

diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
index 5aecbb9fdc68..0978834e22ad 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
@@ -92,4 +92,31 @@ int kfunc_call_test_pass(struct __sk_buff *skb)
return 0;
}

+struct syscall_test_args {
+ __u8 data[16];
+ size_t size;
+};
+
+SEC("syscall")
+int kfunc_syscall_test(struct syscall_test_args *args)
+{
+ const int size = args->size;
+
+ if (size > sizeof(args->data))
+ return -7; /* -E2BIG */
+
+ bpf_kfunc_call_test_mem_len_pass1(&args->data, sizeof(args->data));
+ bpf_kfunc_call_test_mem_len_pass1(&args->data, size);
+
+ return 0;
+}
+
+SEC("syscall")
+int kfunc_syscall_test_fail(struct syscall_test_args *args)
+{
+ bpf_kfunc_call_test_mem_len_pass1(&args->data, sizeof(*args) + 1);
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
--
2.36.1

2022-07-12 15:35:45

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 07/23] bpf: prepare for more bpf syscall to be used from kernel and user space.

Add BPF_MAP_GET_FD_BY_ID and BPF_MAP_DELETE_PROG.

Only BPF_MAP_GET_FD_BY_ID needs to be amended to be able
to access the bpf pointer either from the userspace or the kernel.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

changes in v6:
- commit description change

new in v5
---
kernel/bpf/syscall.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ab688d85b2c6..637765390c30 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1417,9 +1417,9 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)

#define BPF_MAP_DELETE_ELEM_LAST_FIELD key

-static int map_delete_elem(union bpf_attr *attr)
+static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
{
- void __user *ukey = u64_to_user_ptr(attr->key);
+ bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
int ufd = attr->map_fd;
struct bpf_map *map;
struct fd f;
@@ -1439,7 +1439,7 @@ static int map_delete_elem(union bpf_attr *attr)
goto err_put;
}

- key = __bpf_copy_key(ukey, map->key_size);
+ key = ___bpf_copy_key(ukey, map->key_size);
if (IS_ERR(key)) {
err = PTR_ERR(key);
goto err_put;
@@ -4922,7 +4922,7 @@ static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size)
err = map_update_elem(&attr, uattr);
break;
case BPF_MAP_DELETE_ELEM:
- err = map_delete_elem(&attr);
+ err = map_delete_elem(&attr, uattr);
break;
case BPF_MAP_GET_NEXT_KEY:
err = map_get_next_key(&attr);
@@ -5057,8 +5057,10 @@ BPF_CALL_3(bpf_sys_bpf, int, cmd, union bpf_attr *, attr, u32, attr_size)

switch (cmd) {
case BPF_MAP_CREATE:
+ case BPF_MAP_DELETE_ELEM:
case BPF_MAP_UPDATE_ELEM:
case BPF_MAP_FREEZE:
+ case BPF_MAP_GET_FD_BY_ID:
case BPF_PROG_LOAD:
case BPF_BTF_LOAD:
case BPF_LINK_CREATE:
--
2.36.1

2022-07-12 15:39:14

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 15/23] selftests/bpf/hid: add test to change the report size

Use a different report with a bigger size and ensures we are doing
things properly.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

no changes in v6

new in v5
---
tools/testing/selftests/bpf/prog_tests/hid.c | 60 ++++++++++++++++++++
tools/testing/selftests/bpf/progs/hid.c | 15 ++++-
2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index 719d220c8d86..47bc0a30c275 100644
--- a/tools/testing/selftests/bpf/prog_tests/hid.c
+++ b/tools/testing/selftests/bpf/prog_tests/hid.c
@@ -17,6 +17,17 @@ static unsigned char rdesc[] = {
0xa1, 0x01, /* COLLECTION (Application) */
0x09, 0x01, /* Usage (Vendor Usage 0x01) */
0xa1, 0x00, /* COLLECTION (Physical) */
+ 0x85, 0x02, /* REPORT_ID (2) */
+ 0x19, 0x01, /* USAGE_MINIMUM (1) */
+ 0x29, 0x08, /* USAGE_MAXIMUM (3) */
+ 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
+ 0x25, 0xff, /* LOGICAL_MAXIMUM (255) */
+ 0x95, 0x08, /* REPORT_COUNT (8) */
+ 0x75, 0x08, /* REPORT_SIZE (8) */
+ 0x81, 0x02, /* INPUT (Data,Var,Abs) */
+ 0xc0, /* END_COLLECTION */
+ 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) */
@@ -635,6 +646,53 @@ static int test_attach_detach(int uhid_fd, int dev_id)
return ret;
}

+/*
+ * Attach hid_change_report_id 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_change_report(int uhid_fd, int dev_id)
+{
+ struct test_params params;
+ int err, hidraw_fd = -1;
+ u8 buf[10] = {0};
+ int ret = -1;
+
+ err = prep_test(dev_id, "hid_change_report_id", &params);
+ if (!ASSERT_EQ(err, 0, "prep_test(hid_change_report_id)"))
+ goto cleanup;
+
+ hidraw_fd = params.hidraw_fd;
+
+ /* inject one event */
+ buf[0] = 1;
+ buf[1] = 42;
+ send_event(uhid_fd, buf, 6);
+
+ /* read the data from hidraw */
+ memset(buf, 0, sizeof(buf));
+ err = read(hidraw_fd, buf, sizeof(buf));
+ if (!ASSERT_EQ(err, 9, "read_hidraw"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[0], 2, "hid_change_report_id"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[1], 42, "hid_change_report_id"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(buf[2], 0, "leftovers_from_previous_test"))
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ cleanup_test(&params);
+
+ return ret;
+}
+
void serial_test_hid_bpf(void)
{
int err, uhid_fd;
@@ -660,6 +718,8 @@ void serial_test_hid_bpf(void)
ASSERT_OK(err, "hid");
err = test_attach_detach(uhid_fd, dev_id);
ASSERT_OK(err, "hid_attach_detach");
+ err = test_hid_change_report(uhid_fd, dev_id);
+ ASSERT_OK(err, "hid_change_report");

destroy(uhid_fd);

diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
index fc0a4241643a..ee7529c47ad8 100644
--- a/tools/testing/selftests/bpf/progs/hid.c
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -32,7 +32,20 @@ int BPF_PROG(hid_first_event, struct hid_bpf_ctx *hid_ctx)

rw_data[2] = rw_data[1] + 5;

- return 0;
+ return hid_ctx->size;
+}
+
+SEC("?fmod_ret/hid_bpf_device_event")
+int BPF_PROG(hid_change_report_id, struct hid_bpf_ctx *hid_ctx)
+{
+ __u8 *rw_data = hid_bpf_get_data(hid_ctx, 0 /* offset */, 3 /* size */);
+
+ if (!rw_data)
+ return 0; /* EPERM check */
+
+ rw_data[0] = 2;
+
+ return 9;
}

SEC("syscall")
--
2.36.1

2022-07-12 15:41:25

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 06/23] selftests/bpf: Add tests for kfunc returning a memory pointer

We add 2 new kfuncs that are following the RET_PTR_TO_MEM
capability from the previous commit.
Then we test them in selftests:
the first tests are testing valid case, and are not failing,
and the later ones are actually preventing the program to be loaded
because they are wrong.

To work around that, we mark the failing ones as not autoloaded
(with SEC("?tc")), and we manually enable them one by one, ensuring
the verifier rejects them.

To be able to use bpf_program__set_autoload() from libbpf, we need
to use a plain skeleton, not a light-skeleton, and this is why we
also change the Makefile to generate both for kfunc_call_test.c

Signed-off-by: Benjamin Tissoires <[email protected]>

---

new in v6
---
include/linux/btf.h | 4 +-
net/bpf/test_run.c | 22 +++++
tools/testing/selftests/bpf/Makefile | 5 +-
.../selftests/bpf/prog_tests/kfunc_call.c | 48 ++++++++++
.../selftests/bpf/progs/kfunc_call_test.c | 89 +++++++++++++++++++
5 files changed, 165 insertions(+), 3 deletions(-)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index 31da4273c2ec..6f46ff2128ae 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -422,7 +422,9 @@ static inline int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dt

static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type *t)
{
- /* t comes in already as a pointer */
+ if (!btf_type_is_ptr(t))
+ return false;
+
t = btf_type_by_id(btf, t->type);

/* allow const */
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 9da2a42811e8..0b4026ea4652 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -606,6 +606,24 @@ noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p)
WARN_ON_ONCE(1);
}

+static int *__bpf_kfunc_call_test_get_mem(struct prog_test_ref_kfunc *p, const int size)
+{
+ if (size > 2 * sizeof(int))
+ return NULL;
+
+ return (int *)p;
+}
+
+noinline int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size)
+{
+ return __bpf_kfunc_call_test_get_mem(p, rdwr_buf_size);
+}
+
+noinline int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
+{
+ return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
+}
+
noinline struct prog_test_ref_kfunc *
bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b)
{
@@ -704,6 +722,8 @@ BTF_ID(func, bpf_kfunc_call_memb_acquire)
BTF_ID(func, bpf_kfunc_call_test_release)
BTF_ID(func, bpf_kfunc_call_memb_release)
BTF_ID(func, bpf_kfunc_call_memb1_release)
+BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
+BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
BTF_ID(func, bpf_kfunc_call_test_kptr_get)
BTF_ID(func, bpf_kfunc_call_test_pass_ctx)
BTF_ID(func, bpf_kfunc_call_test_pass1)
@@ -731,6 +751,8 @@ BTF_SET_END(test_sk_release_kfunc_ids)
BTF_SET_START(test_sk_ret_null_kfunc_ids)
BTF_ID(func, bpf_kfunc_call_test_acquire)
BTF_ID(func, bpf_kfunc_call_memb_acquire)
+BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
+BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
BTF_ID(func, bpf_kfunc_call_test_kptr_get)
BTF_SET_END(test_sk_ret_null_kfunc_ids)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 8d59ec7f4c2d..0905315ff86d 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -350,11 +350,12 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \
test_subskeleton.skel.h test_subskeleton_lib.skel.h \
test_usdt.skel.h

-LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \
+LSKELS := fentry_test.c fexit_test.c fexit_sleep.c \
test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \
map_ptr_kern.c core_kern.c core_kern_overflow.c
# Generate both light skeleton and libbpf skeleton for these
-LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test_subprog.c
+LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test.c \
+ kfunc_call_test_subprog.c
SKEL_BLACKLIST += $$(LSKELS)

test_static_linked.skel.h-deps := test_static_linked1.o test_static_linked2.o
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
index 22547aafdd60..021ec38562d2 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2021 Facebook */
#include <test_progs.h>
#include <network_helpers.h>
+#include "kfunc_call_test.skel.h"
#include "kfunc_call_test.lskel.h"
#include "kfunc_call_test_subprog.skel.h"
#include "kfunc_call_test_subprog.lskel.h"
@@ -50,10 +51,12 @@ static void test_main(void)
prog_fd = skel->progs.kfunc_syscall_test.prog_fd;
err = bpf_prog_test_run_opts(prog_fd, &syscall_topts);
ASSERT_OK(err, "bpf_prog_test_run(syscall_test)");
+ ASSERT_EQ(syscall_topts.retval, 0, "syscall_test-retval");

prog_fd = skel->progs.kfunc_syscall_test_fail.prog_fd;
err = bpf_prog_test_run_opts(prog_fd, &syscall_topts);
ASSERT_ERR(err, "bpf_prog_test_run(syscall_test_fail)");
+ ASSERT_EQ(syscall_topts.retval, 0, "syscall_test_fail-retval");

kfunc_call_test_lskel__destroy(skel);
}
@@ -106,6 +109,48 @@ static void test_subprog_lskel(void)
kfunc_call_test_subprog_lskel__destroy(skel);
}

+static void test_get_mem(void)
+{
+ struct kfunc_call_test *skel;
+ int prog_fd, err;
+ LIBBPF_OPTS(bpf_test_run_opts, topts,
+ .data_in = &pkt_v4,
+ .data_size_in = sizeof(pkt_v4),
+ .repeat = 1,
+ );
+
+ skel = kfunc_call_test__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel"))
+ return;
+
+ prog_fd = bpf_program__fd(skel->progs.kfunc_call_test_get_mem);
+ err = bpf_prog_test_run_opts(prog_fd, &topts);
+ ASSERT_OK(err, "bpf_prog_test_run(test_get_mem)");
+ ASSERT_EQ(topts.retval, 42, "test_get_mem-retval");
+
+ kfunc_call_test__destroy(skel);
+
+ /* start the various failing tests */
+ skel = kfunc_call_test__open();
+ if (!ASSERT_OK_PTR(skel, "skel"))
+ return;
+
+ bpf_program__set_autoload(skel->progs.kfunc_call_test_get_mem_fail1, true);
+ err = kfunc_call_test__load(skel);
+ ASSERT_ERR(err, "load(kfunc_call_test_get_mem_fail1)");
+ kfunc_call_test__destroy(skel);
+
+ skel = kfunc_call_test__open();
+ if (!ASSERT_OK_PTR(skel, "skel"))
+ return;
+
+ bpf_program__set_autoload(skel->progs.kfunc_call_test_get_mem_fail2, true);
+ err = kfunc_call_test__load(skel);
+ ASSERT_ERR(err, "load(kfunc_call_test_get_mem_fail2)");
+
+ kfunc_call_test__destroy(skel);
+}
+
void test_kfunc_call(void)
{
if (test__start_subtest("main"))
@@ -116,4 +161,7 @@ void test_kfunc_call(void)

if (test__start_subtest("subprog_lskel"))
test_subprog_lskel();
+
+ if (test__start_subtest("get_mem"))
+ test_get_mem();
}
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
index 0978834e22ad..e865f8db26a3 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
@@ -14,6 +14,8 @@ extern void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
extern void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
extern void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;
+extern int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
+extern int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;

SEC("tc")
int kfunc_call_test2(struct __sk_buff *skb)
@@ -119,4 +121,91 @@ int kfunc_syscall_test_fail(struct syscall_test_args *args)
return 0;
}

+SEC("tc")
+int kfunc_call_test_get_mem(struct __sk_buff *skb)
+{
+ struct prog_test_ref_kfunc *pt;
+ unsigned long s = 0;
+ int *p = NULL;
+ int ret = 0;
+
+ pt = bpf_kfunc_call_test_acquire(&s);
+ if (pt) {
+ if (pt->a != 42 || pt->b != 108)
+ ret = -1;
+
+ p = bpf_kfunc_call_test_get_rdwr_mem(pt, 2 * sizeof(int));
+ if (p) {
+ p[0] = 42;
+ ret = p[1]; /* 108 */
+ } else {
+ ret = -1;
+ }
+
+ if (ret >= 0) {
+ p = bpf_kfunc_call_test_get_rdonly_mem(pt, 2 * sizeof(int));
+ if (p)
+ ret = p[0]; /* 42 */
+ else
+ ret = -1;
+ }
+
+ bpf_kfunc_call_test_release(pt);
+ }
+ return ret;
+}
+
+SEC("?tc")
+int kfunc_call_test_get_mem_fail1(struct __sk_buff *skb)
+{
+ struct prog_test_ref_kfunc *pt;
+ unsigned long s = 0;
+ int *p = NULL;
+ int ret = 0;
+
+ pt = bpf_kfunc_call_test_acquire(&s);
+ if (pt) {
+ if (pt->a != 42 || pt->b != 108)
+ ret = -1;
+
+ p = bpf_kfunc_call_test_get_rdonly_mem(pt, 2 * sizeof(int));
+ if (p)
+ p[0] = 42; /* this is a read-only buffer, so -EACCES */
+ else
+ ret = -1;
+
+ bpf_kfunc_call_test_release(pt);
+ }
+ return ret;
+}
+
+SEC("?tc")
+int kfunc_call_test_get_mem_fail2(struct __sk_buff *skb)
+{
+ struct prog_test_ref_kfunc *pt;
+ unsigned long s = 0;
+ int *p = NULL;
+ int ret = 0;
+
+ pt = bpf_kfunc_call_test_acquire(&s);
+ if (pt) {
+ if (pt->a != 42 || pt->b != 108)
+ ret = -1;
+
+ p = bpf_kfunc_call_test_get_rdwr_mem(pt, 2 * sizeof(int));
+ if (p) {
+ p[0] = 42;
+ ret = p[1]; /* 108 */
+ } else {
+ ret = -1;
+ }
+
+ bpf_kfunc_call_test_release(pt);
+ }
+ if (p)
+ ret = p[0]; /* p is not valid anymore */
+
+ return ret;
+}
+
char _license[] SEC("license") = "GPL";
--
2.36.1

2022-07-12 15:46:09

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 01/23] selftests/bpf: fix config for CLS_BPF

When running test_progs under the VM (with vmtest.sh), some tests are
failing because the classifier bpf is not available.

Given that the script doesn't load that particular module, make it
part of vmlinux.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

new in v6
---
tools/testing/selftests/bpf/config | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index c05904d631ec..c69c119f4bb7 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -1,6 +1,6 @@
CONFIG_BPF=y
CONFIG_BPF_SYSCALL=y
-CONFIG_NET_CLS_BPF=m
+CONFIG_NET_CLS_BPF=y
CONFIG_BPF_EVENTS=y
CONFIG_TEST_BPF=m
CONFIG_CGROUP_BPF=y
--
2.36.1

2022-07-12 15:46:30

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 14/23] HID: bpf: allocate data memory for device_event BPF programs

We need to also be able to change the size of the report.
Reducing it is easy, because we already have the incoming buffer that is
big enough, but extending it is harder.

Pre-allocate a buffer that is big enough to handle all reports of the
device, and use that as the primary buffer for BPF programs.
To be able to change the size of the buffer, we change the device_event
API and request it to return the size of the buffer.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

no changes in v6

new-ish in v5
---
drivers/hid/bpf/hid_bpf_dispatch.c | 116 +++++++++++++++++++++++++---
drivers/hid/bpf/hid_bpf_jmp_table.c | 4 +-
drivers/hid/hid-core.c | 12 ++-
include/linux/hid_bpf.h | 37 +++++++--
4 files changed, 151 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 574e0a627861..87fd11539213 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -28,8 +28,9 @@ EXPORT_SYMBOL(hid_bpf_ops);
*
* @ctx: The HID-BPF context
*
- * @return %0 on success and keep processing; a negative error code to interrupt
- * the processing of this event
+ * @return %0 on success and keep processing; a positive value to change the
+ * incoming size buffer; a negative error code to interrupt the processing
+ * of this event
*
* Declare an %fmod_ret tracing bpf program to this function and attach this
* program through hid_bpf_attach_prog() to have this helper called for
@@ -44,23 +45,43 @@ __weak noinline int hid_bpf_device_event(struct hid_bpf_ctx *ctx)
}
ALLOW_ERROR_INJECTION(hid_bpf_device_event, ERRNO);

-int
+u8 *
dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type, u8 *data,
- u32 size, int interrupt)
+ u32 *size, int interrupt)
{
struct hid_bpf_ctx_kern ctx_kern = {
.ctx = {
.hid = hdev,
.report_type = type,
- .size = size,
+ .allocated_size = hdev->bpf.allocated_data,
+ .size = *size,
},
- .data = data,
+ .data = hdev->bpf.device_data,
};
+ int ret;

if (type >= HID_REPORT_TYPES)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+
+ /* no program has been attached yet */
+ if (!hdev->bpf.device_data)
+ return data;
+
+ memset(ctx_kern.data, 0, hdev->bpf.allocated_data);
+ memcpy(ctx_kern.data, data, *size);
+
+ ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_DEVICE_EVENT, &ctx_kern);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ if (ret) {
+ if (ret > ctx_kern.ctx.allocated_size)
+ return ERR_PTR(-EINVAL);

- return hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_DEVICE_EVENT, &ctx_kern);
+ *size = ret;
+ }
+
+ return ctx_kern.data;
}
EXPORT_SYMBOL_GPL(dispatch_hid_bpf_device_event);

@@ -83,7 +104,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr

ctx_kern = container_of(ctx, struct hid_bpf_ctx_kern, ctx);

- if (rdwr_buf_size + offset > ctx->size)
+ if (rdwr_buf_size + offset > ctx->allocated_size)
return NULL;

return ctx_kern->data + offset;
@@ -122,6 +143,51 @@ static int device_match_id(struct device *dev, const void *id)
return hdev->id == *(int *)id;
}

+static int __hid_bpf_allocate_data(struct hid_device *hdev, u8 **data, u32 *size)
+{
+ u8 *alloc_data;
+ unsigned int i, j, max_report_len = 0;
+ size_t alloc_size = 0;
+
+ /* compute the maximum report length for this device */
+ for (i = 0; i < HID_REPORT_TYPES; i++) {
+ struct hid_report_enum *report_enum = hdev->report_enum + i;
+
+ for (j = 0; j < HID_MAX_IDS; j++) {
+ struct hid_report *report = report_enum->report_id_hash[j];
+
+ if (report)
+ max_report_len = max(max_report_len, hid_report_len(report));
+ }
+ }
+
+ /*
+ * Give us a little bit of extra space and some predictability in the
+ * buffer length we create. This way, we can tell users that they can
+ * work on chunks of 64 bytes of memory without having the bpf verifier
+ * scream at them.
+ */
+ alloc_size = DIV_ROUND_UP(max_report_len, 64) * 64;
+
+ alloc_data = kzalloc(alloc_size, GFP_KERNEL);
+ if (!alloc_data)
+ return -ENOMEM;
+
+ *data = alloc_data;
+ *size = alloc_size;
+
+ return 0;
+}
+
+static int hid_bpf_allocate_event_data(struct hid_device *hdev)
+{
+ /* hdev->bpf.device_data is already allocated, abort */
+ if (hdev->bpf.device_data)
+ return 0;
+
+ return __hid_bpf_allocate_data(hdev, &hdev->bpf.device_data, &hdev->bpf.allocated_data);
+}
+
/**
* hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
*
@@ -137,7 +203,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
{
struct hid_device *hdev;
struct device *dev;
- int prog_type = hid_bpf_get_prog_attach_type(prog_fd);
+ int err, prog_type = hid_bpf_get_prog_attach_type(prog_fd);

if (!hid_bpf_ops)
return -EINVAL;
@@ -157,6 +223,12 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)

hdev = to_hid_device(dev);

+ if (prog_type == HID_BPF_PROG_TYPE_DEVICE_EVENT) {
+ err = hid_bpf_allocate_event_data(hdev);
+ if (err)
+ return err;
+ }
+
return __hid_bpf_attach_prog(hdev, prog_type, prog_fd, flags);
}

@@ -170,6 +242,30 @@ static const struct btf_kfunc_id_set hid_bpf_syscall_kfunc_set = {
.check_set = &hid_bpf_syscall_kfunc_ids,
};

+int hid_bpf_connect_device(struct hid_device *hdev)
+{
+ struct hid_bpf_prog_list *prog_list;
+
+ rcu_read_lock();
+ prog_list = rcu_dereference(hdev->bpf.progs[HID_BPF_PROG_TYPE_DEVICE_EVENT]);
+ rcu_read_unlock();
+
+ /* only allocate BPF data if there are programs attached */
+ if (!prog_list)
+ return 0;
+
+ return hid_bpf_allocate_event_data(hdev);
+}
+EXPORT_SYMBOL_GPL(hid_bpf_connect_device);
+
+void hid_bpf_disconnect_device(struct hid_device *hdev)
+{
+ kfree(hdev->bpf.device_data);
+ hdev->bpf.device_data = NULL;
+ hdev->bpf.allocated_data = 0;
+}
+EXPORT_SYMBOL_GPL(hid_bpf_disconnect_device);
+
void hid_bpf_destroy_device(struct hid_device *hdev)
{
if (!hdev)
diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index 05225ff3cc27..0f20deab81ff 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -123,8 +123,10 @@ int hid_bpf_prog_run(struct hid_device *hdev, enum hid_bpf_prog_type type,

ctx_kern->ctx.index = idx;
err = __hid_bpf_tail_call(&ctx_kern->ctx);
- if (err)
+ if (err < 0)
break;
+ if (err)
+ ctx_kern->ctx.retval = err;
}

out_unlock:
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 47cc288fde63..c2589106ea4b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2040,9 +2040,11 @@ int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data
report_enum = hid->report_enum + type;
hdrv = hid->driver;

- ret = dispatch_hid_bpf_device_event(hid, type, data, size, interrupt);
- if (ret)
+ data = dispatch_hid_bpf_device_event(hid, type, data, &size, interrupt);
+ if (IS_ERR(data)) {
+ ret = PTR_ERR(data);
goto unlock;
+ }

if (!size) {
dbg_hid("empty report\n");
@@ -2157,6 +2159,10 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
int len;
int ret;

+ ret = hid_bpf_connect_device(hdev);
+ if (ret)
+ return ret;
+
if (hdev->quirks & HID_QUIRK_HIDDEV_FORCE)
connect_mask |= (HID_CONNECT_HIDDEV_FORCE | HID_CONNECT_HIDDEV);
if (hdev->quirks & HID_QUIRK_HIDINPUT_FORCE)
@@ -2258,6 +2264,8 @@ void hid_disconnect(struct hid_device *hdev)
if (hdev->claimed & HID_CLAIMED_HIDRAW)
hidraw_disconnect(hdev);
hdev->claimed = 0;
+
+ hid_bpf_disconnect_device(hdev);
}
EXPORT_SYMBOL_GPL(hid_disconnect);

diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index 9d893c14a0f2..c9684de18f3f 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -29,15 +29,32 @@ struct hid_device;
* a bigger index).
* @hid: the ``struct hid_device`` representing the device itself
* @report_type: used for ``hid_bpf_device_event()``
+ * @allocated_size: Allocated size of data.
+ *
+ * This is how much memory is available and can be requested
+ * by the HID program.
+ * Note that for ``HID_BPF_RDESC_FIXUP``, that memory is set to
+ * ``4096`` (4 KB)
* @size: Valid data in the data field.
*
* Programs can get the available valid size in data by fetching this field.
+ * Programs can also change this value by returning a positive number in the
+ * program.
+ * To discard the event, return a negative error code.
+ *
+ * ``size`` must always be less or equal than ``allocated_size`` (it is enforced
+ * once all BPF programs have been run).
+ * @retval: Return value of the previous program.
*/
struct hid_bpf_ctx {
__u32 index;
const struct hid_device *hid;
+ __u32 allocated_size;
enum hid_report_type report_type;
- __s32 size;
+ union {
+ __s32 retval;
+ __s32 size;
+ };
};

/* Following functions are tracepoints that BPF programs can attach to */
@@ -78,6 +95,12 @@ struct hid_bpf_prog_list {

/* stored in each device */
struct hid_bpf {
+ u8 *device_data; /* allocated when a bpf program of type
+ * SEC(f.../hid_bpf_device_event) has been attached
+ * to this HID device
+ */
+ u32 allocated_data;
+
struct hid_bpf_prog_list __rcu *progs[HID_BPF_PROG_TYPE_MAX]; /* attached BPF progs */
bool destroyed; /* prevents the assignment of any progs */

@@ -85,13 +108,17 @@ struct hid_bpf {
};

#ifdef CONFIG_BPF
-int dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
- u32 size, int interrupt);
+u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
+ u32 *size, int interrupt);
+int hid_bpf_connect_device(struct hid_device *hdev);
+void hid_bpf_disconnect_device(struct hid_device *hdev);
void hid_bpf_destroy_device(struct hid_device *hid);
void hid_bpf_device_init(struct hid_device *hid);
#else /* CONFIG_BPF */
-static inline int dispatch_hid_bpf_device_event(struct hid_device *hid, int type, u8 *data,
- u32 size, int interrupt) { return 0; }
+static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, int type, u8 *data,
+ u32 *size, int interrupt) { return 0; }
+static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; }
+static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {}
static inline void hid_bpf_destroy_device(struct hid_device *hid) {}
static inline void hid_bpf_device_init(struct hid_device *hid) {}
#endif /* CONFIG_BPF */
--
2.36.1

2022-07-12 15:55:38

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 19/23] selftests/bpf: add report descriptor fixup tests

Simple report descriptor override in HID: replace part of the report
descriptor from a static definition in the bpf kernel program.

Note that this test should be run last because we disconnect/reconnect
the device, meaning that it changes the overall uhid device.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

no changes in v6

changes in v5:
- amended for the new API

not in v4

changes in v3:
- added a comment to mention that this test needs to be run last

changes in v2:
- split the series by bpf/libbpf/hid/selftests and samples
---
tools/testing/selftests/bpf/prog_tests/hid.c | 76 ++++++++++++++++++++
tools/testing/selftests/bpf/progs/hid.c | 53 ++++++++++++++
2 files changed, 129 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/hid.c b/tools/testing/selftests/bpf/prog_tests/hid.c
index 19172d3e0f44..9dc5f0038472 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[] = {
@@ -769,6 +770,73 @@ static int test_hid_user_raw_request_call(int uhid_fd, int dev_id)
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(void)
+{
+ struct hidraw_report_descriptor rpt_desc = {0};
+ struct test_params params;
+ int err, uhid_fd, desc_size, hidraw_fd = -1, ret = -1;
+ int dev_id;
+ void *uhid_err;
+ pthread_t tid;
+ bool started = false;
+
+ dev_id = rand() % 1024;
+
+ uhid_fd = setup_uhid(dev_id);
+ if (!ASSERT_GE(uhid_fd, 0, "setup uhid"))
+ return uhid_fd;
+
+ err = prep_test(dev_id, "hid_rdesc_fixup", &params);
+ if (!ASSERT_EQ(err, 0, "prep_test(hid_rdesc_fixup)"))
+ goto cleanup;
+
+ err = uhid_start_listener(&tid, uhid_fd);
+ ASSERT_OK(err, "uhid_start_listener");
+
+ started = true;
+
+ hidraw_fd = params.hidraw_fd;
+
+ /* check that hid_rdesc_fixup() was executed */
+ ASSERT_EQ(params.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:
+ cleanup_test(&params);
+
+ if (started) {
+ destroy(uhid_fd);
+ pthread_join(tid, &uhid_err);
+ err = (int)(long)uhid_err;
+ CHECK_FAIL(err);
+ }
+
+ return ret;
+}
+
void serial_test_hid_bpf(void)
{
int err, uhid_fd;
@@ -799,6 +867,14 @@ void serial_test_hid_bpf(void)
err = test_hid_user_raw_request_call(uhid_fd, dev_id);
ASSERT_OK(err, "hid_user_raw_request");

+ /*
+ * this test should be run last because we disconnect/reconnect
+ * the device, meaning that it changes the overall uhid device
+ * and messes up with the thread that reads uhid events.
+ */
+ err = test_rdesc_fixup();
+ ASSERT_OK(err, "hid_rdesc_fixup");
+
destroy(uhid_fd);

pthread_join(tid, &uhid_err);
diff --git a/tools/testing/selftests/bpf/progs/hid.c b/tools/testing/selftests/bpf/progs/hid.c
index 0be319c11575..863c37ddf5ff 100644
--- a/tools/testing/selftests/bpf/progs/hid.c
+++ b/tools/testing/selftests/bpf/progs/hid.c
@@ -99,3 +99,56 @@ int hid_user_raw_request(struct hid_hw_request_syscall_args *args)

return 0;
}
+
+static const __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("?fmod_ret/hid_bpf_rdesc_fixup")
+int BPF_PROG(hid_rdesc_fixup, struct hid_bpf_ctx *hid_ctx)
+{
+ __u8 *data = hid_bpf_get_data(hid_ctx, 0 /* offset */, 4096 /* size */);
+
+ if (!data)
+ return 0; /* EPERM check */
+
+ callback2_check = data[4];
+
+ /* insert rdesc at offset 73 */
+ __builtin_memcpy(&data[73], rdesc, sizeof(rdesc));
+
+ /* Change Usage Vendor globally */
+ data[4] = 0x42;
+
+ return sizeof(rdesc) + 73;
+}
--
2.36.1

2022-07-12 15:56:12

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH bpf-next v6 09/23] HID: core: store the unique system identifier in hid_device

This unique identifier is currently used only for ensuring uniqueness in
sysfs. However, this could be handful for userspace to refer to a specific
hid_device by this id.

2 use cases are in my mind: LEDs (and their naming convention), and
HID-BPF.

Signed-off-by: Benjamin Tissoires <[email protected]>

---

no changes in v6

new in v5
---
drivers/hid/hid-core.c | 4 +++-
include/linux/hid.h | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 00154a1cd2d8..11874d264728 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2739,10 +2739,12 @@ int hid_add_device(struct hid_device *hdev)
hid_warn(hdev, "bad device descriptor (%d)\n", ret);
}

+ hdev->id = atomic_inc_return(&id);
+
/* XXX hack, any other cleaner solution after the driver core
* is converted to allow more than 20 bytes as the device name? */
dev_set_name(&hdev->dev, "%04X:%04X:%04X.%04X", hdev->bus,
- hdev->vendor, hdev->product, atomic_inc_return(&id));
+ hdev->vendor, hdev->product, hdev->id);

hid_debug_register(hdev, dev_name(&hdev->dev));
ret = device_add(&hdev->dev);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 4363a63b9775..a43dd17bc78f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -658,6 +658,8 @@ struct hid_device { /* device report descriptor */
struct list_head debug_list;
spinlock_t debug_list_lock;
wait_queue_head_t debug_wait;
+
+ unsigned int id; /* system unique id */
};

#define to_hid_device(pdev) \
--
2.36.1

2022-07-13 12:08:57

by Dominique Martinet

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 21/23] samples/bpf: add new hid_mouse example

Benjamin Tissoires wrote on Tue, Jul 12, 2022 at 04:58:48PM +0200:
> diff --git a/samples/bpf/hid_mouse.c b/samples/bpf/hid_mouse.c
> new file mode 100644
> index 000000000000..f6e5f09026eb
> --- /dev/null
> +++ b/samples/bpf/hid_mouse.c
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2022 Benjamin Tissoires
> + */
> +
> +/* not sure why but this doesn't get preoperly imported */

typo: properly

> +#define __must_check

But more usefully, I don't think it should be needed -- we don't use
__must_check at all in uapi includes; if this is needed that means some
of the include here uses the kernel internal includes and that shouldn't
be needed as they're not normally installed.

Didn't actually try to see but taking the compilation line that fails
and running it with -E will probably show where that must_check comes
from

--
Dominique Martinet | Asmadeus,
just passing by

2022-07-13 12:40:36

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 21/23] samples/bpf: add new hid_mouse example

On Wed, Jul 13, 2022 at 2:16 PM Dominique Martinet
<[email protected]> wrote:
>
> Benjamin Tissoires wrote on Tue, Jul 12, 2022 at 04:58:48PM +0200:
> > diff --git a/samples/bpf/hid_mouse.c b/samples/bpf/hid_mouse.c
> > new file mode 100644
> > index 000000000000..f6e5f09026eb
> > --- /dev/null
> > +++ b/samples/bpf/hid_mouse.c
> > @@ -0,0 +1,150 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright (c) 2022 Benjamin Tissoires
> > + */
> > +
> > +/* not sure why but this doesn't get preoperly imported */
>
> typo: properly
>
> > +#define __must_check
>
> But more usefully, I don't think it should be needed -- we don't use
> __must_check at all in uapi includes; if this is needed that means some
> of the include here uses the kernel internal includes and that shouldn't
> be needed as they're not normally installed.

Indeed, I must have had the issue in the early days of development.
Removing the line still makes the program compile, so I'll remove it
in v7.

Thanks a lot!

Cheers,
Benjamin

>
> Didn't actually try to see but taking the compilation line that fails
> and running it with -E will probably show where that must_check comes
> from
>
> --
> Dominique Martinet | Asmadeus,
> just passing by
>

2022-07-14 22:09:56

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 00/23] Introduce eBPF support for HID devices

On Tue, Jul 12, 2022 at 04:58:27PM +0200, Benjamin Tissoires wrote:
> Hi,
>
> and after a little bit of time, here comes the v6 of the HID-BPF series.
>
> Again, for a full explanation of HID-BPF, please refer to the last patch
> in this series (23/23).
>
> This version sees some improvements compared to v5 on top of the
> usual addressing of the previous comments:
> - now I think every eBPF core change has a matching selftest added
> - the kfuncs declared in syscall can now actually access the memory of
> the context
> - the code to retrieve the BTF ID of the various HID hooks is much
> simpler (just a plain use of the BTF_ID() API instead of
> loading/unloading of a tracing program)
> - I also added my HID Surface Dial example that I use locally to provide
> a fuller example to users

Looking great.
Before another respin to address bits in patch 12 let's land the first ~8 patches,
since they're generic useful improvements.

Kumar, could you please help review the verifier bits?

2022-07-15 05:02:14

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 09/23] HID: core: store the unique system identifier in hid_device

On Tue, Jul 12, 2022 at 04:58:36PM +0200, Benjamin Tissoires wrote:
> This unique identifier is currently used only for ensuring uniqueness in
> sysfs. However, this could be handful for userspace to refer to a specific
> hid_device by this id.
>
> 2 use cases are in my mind: LEDs (and their naming convention), and
> HID-BPF.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
>


Reviewed-by: Greg Kroah-Hartman <[email protected]>

2022-07-15 05:03:10

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 11/23] HID: convert defines of HID class requests into a proper enum

On Tue, Jul 12, 2022 at 04:58:38PM +0200, Benjamin Tissoires wrote:
> This allows to export the type in BTF and so in the automatically
> generated vmlinux.h. It will also add some static checks on the users
> when we change the ll driver API (see not below).
>
> Note that we need to also do change in the ll_driver API, but given
> that this will have a wider impact outside of this tree, we leave this
> as a TODO for the future.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>

Reviewed-by: Greg Kroah-Hartman <[email protected]>

2022-07-15 17:56:59

by Kumar Kartikeya Dwivedi

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 00/23] Introduce eBPF support for HID devices

On Thu, 14 Jul 2022 at 23:39, Alexei Starovoitov
<[email protected]> wrote:
>
> On Tue, Jul 12, 2022 at 04:58:27PM +0200, Benjamin Tissoires wrote:
> > Hi,
> >
> > and after a little bit of time, here comes the v6 of the HID-BPF series.
> >
> > Again, for a full explanation of HID-BPF, please refer to the last patch
> > in this series (23/23).
> >
> > This version sees some improvements compared to v5 on top of the
> > usual addressing of the previous comments:
> > - now I think every eBPF core change has a matching selftest added
> > - the kfuncs declared in syscall can now actually access the memory of
> > the context
> > - the code to retrieve the BTF ID of the various HID hooks is much
> > simpler (just a plain use of the BTF_ID() API instead of
> > loading/unloading of a tracing program)
> > - I also added my HID Surface Dial example that I use locally to provide
> > a fuller example to users
>
> Looking great.
> Before another respin to address bits in patch 12 let's land the first ~8 patches,
> since they're generic useful improvements.
>
> Kumar, could you please help review the verifier bits?

Sure, I'll take a look.

2022-07-16 00:22:08

by Yonghong Song

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 01/23] selftests/bpf: fix config for CLS_BPF



On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
> When running test_progs under the VM (with vmtest.sh), some tests are
> failing because the classifier bpf is not available.
>
> Given that the script doesn't load that particular module, make it
> part of vmlinux.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>

Acked-by: Yonghong Song <[email protected]>

2022-07-16 00:44:59

by Yonghong Song

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 04/23] selftests/bpf: add test for accessing ctx from syscall program type



On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
> We need to also export the kfunc set to the syscall program type,
> and then add a couple of eBPF programs that are testing those calls.
>
> The first one checks for valid access, and the second one is OK
> from a static analysis point of view but fails at run time because
> we are trying to access outside of the allocated memory.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>

Acked-by: Yonghong Song <[email protected]>

2022-07-16 04:52:03

by Yonghong Song

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 06/23] selftests/bpf: Add tests for kfunc returning a memory pointer



On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
> We add 2 new kfuncs that are following the RET_PTR_TO_MEM
> capability from the previous commit.
> Then we test them in selftests:
> the first tests are testing valid case, and are not failing,
> and the later ones are actually preventing the program to be loaded
> because they are wrong.
>
> To work around that, we mark the failing ones as not autoloaded
> (with SEC("?tc")), and we manually enable them one by one, ensuring
> the verifier rejects them.
>
> To be able to use bpf_program__set_autoload() from libbpf, we need
> to use a plain skeleton, not a light-skeleton, and this is why we
> also change the Makefile to generate both for kfunc_call_test.c
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
>
> ---
>
> new in v6
> ---
> include/linux/btf.h | 4 +-
> net/bpf/test_run.c | 22 +++++
> tools/testing/selftests/bpf/Makefile | 5 +-
> .../selftests/bpf/prog_tests/kfunc_call.c | 48 ++++++++++
> .../selftests/bpf/progs/kfunc_call_test.c | 89 +++++++++++++++++++
> 5 files changed, 165 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 31da4273c2ec..6f46ff2128ae 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -422,7 +422,9 @@ static inline int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dt
>
> static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type *t)
> {
> - /* t comes in already as a pointer */
> + if (!btf_type_is_ptr(t))
> + return false;

Why we have a change here?

> +
> t = btf_type_by_id(btf, t->type);
>
> /* allow const */
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 9da2a42811e8..0b4026ea4652 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -606,6 +606,24 @@ noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p)
> WARN_ON_ONCE(1);
> }
>
> +static int *__bpf_kfunc_call_test_get_mem(struct prog_test_ref_kfunc *p, const int size)
> +{
> + if (size > 2 * sizeof(int))
> + return NULL;
> +
> + return (int *)p;
> +}
> +
> +noinline int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size)
> +{
> + return __bpf_kfunc_call_test_get_mem(p, rdwr_buf_size);
> +}
> +
> +noinline int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
> +{
> + return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
> +}
> +
> noinline struct prog_test_ref_kfunc *
> bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b)
> {
> @@ -704,6 +722,8 @@ BTF_ID(func, bpf_kfunc_call_memb_acquire)
> BTF_ID(func, bpf_kfunc_call_test_release)
> BTF_ID(func, bpf_kfunc_call_memb_release)
> BTF_ID(func, bpf_kfunc_call_memb1_release)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
> BTF_ID(func, bpf_kfunc_call_test_kptr_get)
> BTF_ID(func, bpf_kfunc_call_test_pass_ctx)
> BTF_ID(func, bpf_kfunc_call_test_pass1)
> @@ -731,6 +751,8 @@ BTF_SET_END(test_sk_release_kfunc_ids)
> BTF_SET_START(test_sk_ret_null_kfunc_ids)
> BTF_ID(func, bpf_kfunc_call_test_acquire)
> BTF_ID(func, bpf_kfunc_call_memb_acquire)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
> BTF_ID(func, bpf_kfunc_call_test_kptr_get)
> BTF_SET_END(test_sk_ret_null_kfunc_ids)
>
[...]

2022-07-16 06:15:26

by Yonghong Song

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 07/23] bpf: prepare for more bpf syscall to be used from kernel and user space.



On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
> Add BPF_MAP_GET_FD_BY_ID and BPF_MAP_DELETE_PROG.
>
> Only BPF_MAP_GET_FD_BY_ID needs to be amended to be able
> to access the bpf pointer either from the userspace or the kernel.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>

Acked-by: Yonghong Song <[email protected]>

2022-07-18 08:54:44

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH bpf-next v6 06/23] selftests/bpf: Add tests for kfunc returning a memory pointer

On Sat, Jul 16, 2022 at 6:34 AM Yonghong Song <[email protected]> wrote:
>
>
>
> On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
> > We add 2 new kfuncs that are following the RET_PTR_TO_MEM
> > capability from the previous commit.
> > Then we test them in selftests:
> > the first tests are testing valid case, and are not failing,
> > and the later ones are actually preventing the program to be loaded
> > because they are wrong.
> >
> > To work around that, we mark the failing ones as not autoloaded
> > (with SEC("?tc")), and we manually enable them one by one, ensuring
> > the verifier rejects them.
> >
> > To be able to use bpf_program__set_autoload() from libbpf, we need
> > to use a plain skeleton, not a light-skeleton, and this is why we
> > also change the Makefile to generate both for kfunc_call_test.c
> >
> > Signed-off-by: Benjamin Tissoires <[email protected]>
> >
> > ---
> >
> > new in v6
> > ---
> > include/linux/btf.h | 4 +-
> > net/bpf/test_run.c | 22 +++++
> > tools/testing/selftests/bpf/Makefile | 5 +-
> > .../selftests/bpf/prog_tests/kfunc_call.c | 48 ++++++++++
> > .../selftests/bpf/progs/kfunc_call_test.c | 89 +++++++++++++++++++
> > 5 files changed, 165 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/btf.h b/include/linux/btf.h
> > index 31da4273c2ec..6f46ff2128ae 100644
> > --- a/include/linux/btf.h
> > +++ b/include/linux/btf.h
> > @@ -422,7 +422,9 @@ static inline int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dt
> >
> > static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type *t)
> > {
> > - /* t comes in already as a pointer */
> > + if (!btf_type_is_ptr(t))
> > + return false;
>
> Why we have a change here?

Definitely a mistake while fixing/rebasing the series.

Will bring this hunk in the previous patch in the next revision.

Thanks for the review!

Cheers,
Benjamin

>
> > +
> > t = btf_type_by_id(btf, t->type);
> >
> > /* allow const */
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index 9da2a42811e8..0b4026ea4652 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -606,6 +606,24 @@ noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p)
> > WARN_ON_ONCE(1);
> > }
> >
> > +static int *__bpf_kfunc_call_test_get_mem(struct prog_test_ref_kfunc *p, const int size)
> > +{
> > + if (size > 2 * sizeof(int))
> > + return NULL;
> > +
> > + return (int *)p;
> > +}
> > +
> > +noinline int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size)
> > +{
> > + return __bpf_kfunc_call_test_get_mem(p, rdwr_buf_size);
> > +}
> > +
> > +noinline int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
> > +{
> > + return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
> > +}
> > +
> > noinline struct prog_test_ref_kfunc *
> > bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b)
> > {
> > @@ -704,6 +722,8 @@ BTF_ID(func, bpf_kfunc_call_memb_acquire)
> > BTF_ID(func, bpf_kfunc_call_test_release)
> > BTF_ID(func, bpf_kfunc_call_memb_release)
> > BTF_ID(func, bpf_kfunc_call_memb1_release)
> > +BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
> > +BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
> > BTF_ID(func, bpf_kfunc_call_test_kptr_get)
> > BTF_ID(func, bpf_kfunc_call_test_pass_ctx)
> > BTF_ID(func, bpf_kfunc_call_test_pass1)
> > @@ -731,6 +751,8 @@ BTF_SET_END(test_sk_release_kfunc_ids)
> > BTF_SET_START(test_sk_ret_null_kfunc_ids)
> > BTF_ID(func, bpf_kfunc_call_test_acquire)
> > BTF_ID(func, bpf_kfunc_call_memb_acquire)
> > +BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
> > +BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
> > BTF_ID(func, bpf_kfunc_call_test_kptr_get)
> > BTF_SET_END(test_sk_ret_null_kfunc_ids)
> >
> [...]
>