* [PATCH v2 1/3] HID: bpf: remove double fdget()
2024-01-24 11:26 [PATCH v2 0/3] HID: bpf: couple of upstream fixes Benjamin Tissoires
@ 2024-01-24 11:26 ` Benjamin Tissoires
2024-01-24 11:26 ` [PATCH v2 2/3] HID: bpf: actually free hdev memory after attaching a HID-BPF program Benjamin Tissoires
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Benjamin Tissoires @ 2024-01-24 11:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
Andrii Nakryiko
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires, stable
When the kfunc hid_bpf_attach_prog() is called, we called twice fdget():
one for fetching the type of the bpf program, and one for actually
attaching the program to the device.
The problem is that between those two calls, we have no guarantees that
the prog_fd is still the same file descriptor for the given program.
Solve this by calling bpf_prog_get() earlier, and use this to fetch the
program type.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/bpf/CAO-hwJJ8vh8JD3-P43L-_CLNmPx0hWj44aom0O838vfP4=_1CA@mail.gmail.com/T/#t
Cc: stable@vger.kernel.org
Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
changes in v2:
* add missing Fixes tag in the commit description
---
drivers/hid/bpf/hid_bpf_dispatch.c | 66 ++++++++++++++++++++++++-------------
drivers/hid/bpf/hid_bpf_dispatch.h | 4 +--
drivers/hid/bpf/hid_bpf_jmp_table.c | 20 ++---------
3 files changed, 49 insertions(+), 41 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index d9ef45fcaeab..5111d1fef0d3 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -241,6 +241,39 @@ int hid_bpf_reconnect(struct hid_device *hdev)
return 0;
}
+static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct bpf_prog *prog,
+ __u32 flags)
+{
+ int fd, err, prog_type;
+
+ prog_type = hid_bpf_get_prog_attach_type(prog);
+ if (prog_type < 0)
+ return prog_type;
+
+ if (prog_type >= HID_BPF_PROG_TYPE_MAX)
+ return -EINVAL;
+
+ if (prog_type == HID_BPF_PROG_TYPE_DEVICE_EVENT) {
+ err = hid_bpf_allocate_event_data(hdev);
+ if (err)
+ return err;
+ }
+
+ fd = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, prog, flags);
+ if (fd < 0)
+ return fd;
+
+ if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
+ err = hid_bpf_reconnect(hdev);
+ if (err) {
+ close_fd(fd);
+ return err;
+ }
+ }
+
+ return fd;
+}
+
/**
* hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
*
@@ -257,18 +290,13 @@ noinline int
hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
{
struct hid_device *hdev;
+ struct bpf_prog *prog;
struct device *dev;
- int fd, err, prog_type = hid_bpf_get_prog_attach_type(prog_fd);
+ int fd;
if (!hid_bpf_ops)
return -EINVAL;
- if (prog_type < 0)
- return prog_type;
-
- if (prog_type >= HID_BPF_PROG_TYPE_MAX)
- return -EINVAL;
-
if ((flags & ~HID_BPF_FLAG_MASK))
return -EINVAL;
@@ -278,23 +306,17 @@ 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;
- }
+ /*
+ * take a ref on the prog itself, it will be released
+ * on errors or when it'll be detached
+ */
+ prog = bpf_prog_get(prog_fd);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
- fd = __hid_bpf_attach_prog(hdev, prog_type, prog_fd, flags);
+ fd = do_hid_bpf_attach_prog(hdev, prog_fd, prog, flags);
if (fd < 0)
- return fd;
-
- if (prog_type == HID_BPF_PROG_TYPE_RDESC_FIXUP) {
- err = hid_bpf_reconnect(hdev);
- if (err) {
- close_fd(fd);
- return err;
- }
- }
+ bpf_prog_put(prog);
return fd;
}
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.h b/drivers/hid/bpf/hid_bpf_dispatch.h
index 63dfc8605cd2..fbe0639d09f2 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.h
+++ b/drivers/hid/bpf/hid_bpf_dispatch.h
@@ -12,9 +12,9 @@ struct hid_bpf_ctx_kern {
int hid_bpf_preload_skel(void);
void hid_bpf_free_links_and_skel(void);
-int hid_bpf_get_prog_attach_type(int prog_fd);
+int hid_bpf_get_prog_attach_type(struct bpf_prog *prog);
int __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type, int prog_fd,
- __u32 flags);
+ struct bpf_prog *prog, __u32 flags);
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);
diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index eca34b7372f9..12f7cebddd73 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -333,15 +333,10 @@ static int hid_bpf_insert_prog(int prog_fd, struct bpf_prog *prog)
return err;
}
-int hid_bpf_get_prog_attach_type(int prog_fd)
+int hid_bpf_get_prog_attach_type(struct bpf_prog *prog)
{
- struct bpf_prog *prog = NULL;
- int i;
int prog_type = HID_BPF_PROG_TYPE_UNDEF;
-
- prog = bpf_prog_get(prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
+ int i;
for (i = 0; i < HID_BPF_PROG_TYPE_MAX; i++) {
if (hid_bpf_btf_ids[i] == prog->aux->attach_btf_id) {
@@ -350,8 +345,6 @@ int hid_bpf_get_prog_attach_type(int prog_fd)
}
}
- bpf_prog_put(prog);
-
return prog_type;
}
@@ -388,19 +381,13 @@ static const struct bpf_link_ops hid_bpf_link_lops = {
/* called from syscall */
noinline int
__hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
- int prog_fd, __u32 flags)
+ int prog_fd, struct bpf_prog *prog, __u32 flags)
{
struct bpf_link_primer link_primer;
struct hid_bpf_link *link;
- struct bpf_prog *prog = NULL;
struct hid_bpf_prog_entry *prog_entry;
int cnt, err = -EINVAL, prog_table_idx = -1;
- /* take a ref on the prog itself */
- prog = bpf_prog_get(prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
-
mutex_lock(&hid_bpf_attach_lock);
link = kzalloc(sizeof(*link), GFP_USER);
@@ -467,7 +454,6 @@ __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
err_unlock:
mutex_unlock(&hid_bpf_attach_lock);
- bpf_prog_put(prog);
kfree(link);
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/3] HID: bpf: actually free hdev memory after attaching a HID-BPF program
2024-01-24 11:26 [PATCH v2 0/3] HID: bpf: couple of upstream fixes Benjamin Tissoires
2024-01-24 11:26 ` [PATCH v2 1/3] HID: bpf: remove double fdget() Benjamin Tissoires
@ 2024-01-24 11:26 ` Benjamin Tissoires
2024-01-26 11:20 ` Benjamin Tissoires
2024-01-24 11:26 ` [PATCH v2 3/3] HID: bpf: use __bpf_kfunc instead of noinline Benjamin Tissoires
2024-01-31 10:38 ` [PATCH v2 0/3] HID: bpf: couple of upstream fixes Benjamin Tissoires
3 siblings, 1 reply; 6+ messages in thread
From: Benjamin Tissoires @ 2024-01-24 11:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
Andrii Nakryiko
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires, stable
Turns out that I got my reference counts wrong and each successful
bus_find_device() actually calls get_device(), and we need to manually
call put_device().
Ensure each bus_find_device() gets a matching put_device() when releasing
the bpf programs and fix all the error paths.
Cc: stable@vger.kernel.org
Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
new in v2
---
drivers/hid/bpf/hid_bpf_dispatch.c | 29 +++++++++++++++++++++++------
drivers/hid/bpf/hid_bpf_jmp_table.c | 19 ++++++++++++++++---
2 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 5111d1fef0d3..7903c8638e81 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -292,7 +292,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
struct hid_device *hdev;
struct bpf_prog *prog;
struct device *dev;
- int fd;
+ int err, fd;
if (!hid_bpf_ops)
return -EINVAL;
@@ -311,14 +311,24 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
* on errors or when it'll be detached
*/
prog = bpf_prog_get(prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
+ if (IS_ERR(prog)) {
+ err = PTR_ERR(prog);
+ goto out_dev_put;
+ }
fd = do_hid_bpf_attach_prog(hdev, prog_fd, prog, flags);
- if (fd < 0)
- bpf_prog_put(prog);
+ if (fd < 0) {
+ err = fd;
+ goto out_prog_put;
+ }
return fd;
+
+ out_prog_put:
+ bpf_prog_put(prog);
+ out_dev_put:
+ put_device(dev);
+ return err;
}
/**
@@ -345,8 +355,10 @@ hid_bpf_allocate_context(unsigned int hid_id)
hdev = to_hid_device(dev);
ctx_kern = kzalloc(sizeof(*ctx_kern), GFP_KERNEL);
- if (!ctx_kern)
+ if (!ctx_kern) {
+ put_device(dev);
return NULL;
+ }
ctx_kern->ctx.hid = hdev;
@@ -363,10 +375,15 @@ noinline void
hid_bpf_release_context(struct hid_bpf_ctx *ctx)
{
struct hid_bpf_ctx_kern *ctx_kern;
+ struct hid_device *hid;
ctx_kern = container_of(ctx, struct hid_bpf_ctx_kern, ctx);
+ hid = (struct hid_device *)ctx_kern->ctx.hid; /* ignore const */
kfree(ctx_kern);
+
+ /* get_device() is called by bus_find_device() */
+ put_device(&hid->dev);
}
/**
diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index 12f7cebddd73..85a24bc0ea25 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -196,6 +196,7 @@ static void __hid_bpf_do_release_prog(int map_fd, unsigned int idx)
static void hid_bpf_release_progs(struct work_struct *work)
{
int i, j, n, map_fd = -1;
+ bool hdev_destroyed;
if (!jmp_table.map)
return;
@@ -220,6 +221,12 @@ static void hid_bpf_release_progs(struct work_struct *work)
if (entry->hdev) {
hdev = entry->hdev;
type = entry->type;
+ /*
+ * hdev is still valid, even if we are called after hid_destroy_device():
+ * when hid_bpf_attach() gets called, it takes a ref on the dev through
+ * bus_find_device()
+ */
+ hdev_destroyed = hdev->bpf.destroyed;
hid_bpf_populate_hdev(hdev, type);
@@ -232,12 +239,18 @@ static void hid_bpf_release_progs(struct work_struct *work)
if (test_bit(next->idx, jmp_table.enabled))
continue;
- if (next->hdev == hdev && next->type == type)
+ if (next->hdev == hdev && next->type == type) {
+ /*
+ * clear the hdev reference and decrement the device ref
+ * that was taken during bus_find_device() while calling
+ * hid_bpf_attach()
+ */
next->hdev = NULL;
+ put_device(&hdev->dev);
}
- /* if type was rdesc fixup, reconnect device */
- if (type == HID_BPF_PROG_TYPE_RDESC_FIXUP)
+ /* if type was rdesc fixup and the device is not gone, reconnect device */
+ if (type == HID_BPF_PROG_TYPE_RDESC_FIXUP && !hdev_destroyed)
hid_bpf_reconnect(hdev);
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/3] HID: bpf: actually free hdev memory after attaching a HID-BPF program
2024-01-24 11:26 ` [PATCH v2 2/3] HID: bpf: actually free hdev memory after attaching a HID-BPF program Benjamin Tissoires
@ 2024-01-26 11:20 ` Benjamin Tissoires
0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Tissoires @ 2024-01-26 11:20 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Dan Carpenter, Daniel Borkmann, Andrii Nakryiko,
linux-input, linux-kernel, bpf, stable
On Wed, Jan 24, 2024 at 12:27 PM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> Turns out that I got my reference counts wrong and each successful
> bus_find_device() actually calls get_device(), and we need to manually
> call put_device().
>
> Ensure each bus_find_device() gets a matching put_device() when releasing
> the bpf programs and fix all the error paths.
>
> Cc: stable@vger.kernel.org
> Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
>
> ---
>
> new in v2
> ---
> drivers/hid/bpf/hid_bpf_dispatch.c | 29 +++++++++++++++++++++++------
> drivers/hid/bpf/hid_bpf_jmp_table.c | 19 ++++++++++++++++---
> 2 files changed, 39 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index 5111d1fef0d3..7903c8638e81 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -292,7 +292,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
> struct hid_device *hdev;
> struct bpf_prog *prog;
> struct device *dev;
> - int fd;
> + int err, fd;
>
> if (!hid_bpf_ops)
> return -EINVAL;
> @@ -311,14 +311,24 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
> * on errors or when it'll be detached
> */
> prog = bpf_prog_get(prog_fd);
> - if (IS_ERR(prog))
> - return PTR_ERR(prog);
> + if (IS_ERR(prog)) {
> + err = PTR_ERR(prog);
> + goto out_dev_put;
> + }
>
> fd = do_hid_bpf_attach_prog(hdev, prog_fd, prog, flags);
> - if (fd < 0)
> - bpf_prog_put(prog);
> + if (fd < 0) {
> + err = fd;
> + goto out_prog_put;
> + }
>
> return fd;
> +
> + out_prog_put:
> + bpf_prog_put(prog);
> + out_dev_put:
> + put_device(dev);
> + return err;
> }
>
> /**
> @@ -345,8 +355,10 @@ hid_bpf_allocate_context(unsigned int hid_id)
> hdev = to_hid_device(dev);
>
> ctx_kern = kzalloc(sizeof(*ctx_kern), GFP_KERNEL);
> - if (!ctx_kern)
> + if (!ctx_kern) {
> + put_device(dev);
> return NULL;
> + }
>
> ctx_kern->ctx.hid = hdev;
>
> @@ -363,10 +375,15 @@ noinline void
> hid_bpf_release_context(struct hid_bpf_ctx *ctx)
> {
> struct hid_bpf_ctx_kern *ctx_kern;
> + struct hid_device *hid;
>
> ctx_kern = container_of(ctx, struct hid_bpf_ctx_kern, ctx);
> + hid = (struct hid_device *)ctx_kern->ctx.hid; /* ignore const */
>
> kfree(ctx_kern);
> +
> + /* get_device() is called by bus_find_device() */
> + put_device(&hid->dev);
> }
>
> /**
> diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
> index 12f7cebddd73..85a24bc0ea25 100644
> --- a/drivers/hid/bpf/hid_bpf_jmp_table.c
> +++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
> @@ -196,6 +196,7 @@ static void __hid_bpf_do_release_prog(int map_fd, unsigned int idx)
> static void hid_bpf_release_progs(struct work_struct *work)
> {
> int i, j, n, map_fd = -1;
> + bool hdev_destroyed;
>
> if (!jmp_table.map)
> return;
> @@ -220,6 +221,12 @@ static void hid_bpf_release_progs(struct work_struct *work)
> if (entry->hdev) {
> hdev = entry->hdev;
> type = entry->type;
> + /*
> + * hdev is still valid, even if we are called after hid_destroy_device():
> + * when hid_bpf_attach() gets called, it takes a ref on the dev through
> + * bus_find_device()
> + */
> + hdev_destroyed = hdev->bpf.destroyed;
>
> hid_bpf_populate_hdev(hdev, type);
>
> @@ -232,12 +239,18 @@ static void hid_bpf_release_progs(struct work_struct *work)
> if (test_bit(next->idx, jmp_table.enabled))
> continue;
>
> - if (next->hdev == hdev && next->type == type)
> + if (next->hdev == hdev && next->type == type) {
> + /*
> + * clear the hdev reference and decrement the device ref
> + * that was taken during bus_find_device() while calling
> + * hid_bpf_attach()
> + */
> next->hdev = NULL;
> + put_device(&hdev->dev);
sigh... I can't make a correct patch these days... Missing a '}' here
to match the open bracket added above :(
I had some debug information put there to check if the device was
actually freed, and the closing bracket got lost while cleaning this
up.
Cheers,
Benjamin
> }
>
> - /* if type was rdesc fixup, reconnect device */
> - if (type == HID_BPF_PROG_TYPE_RDESC_FIXUP)
> + /* if type was rdesc fixup and the device is not gone, reconnect device */
> + if (type == HID_BPF_PROG_TYPE_RDESC_FIXUP && !hdev_destroyed)
> hid_bpf_reconnect(hdev);
> }
> }
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] HID: bpf: use __bpf_kfunc instead of noinline
2024-01-24 11:26 [PATCH v2 0/3] HID: bpf: couple of upstream fixes Benjamin Tissoires
2024-01-24 11:26 ` [PATCH v2 1/3] HID: bpf: remove double fdget() Benjamin Tissoires
2024-01-24 11:26 ` [PATCH v2 2/3] HID: bpf: actually free hdev memory after attaching a HID-BPF program Benjamin Tissoires
@ 2024-01-24 11:26 ` Benjamin Tissoires
2024-01-31 10:38 ` [PATCH v2 0/3] HID: bpf: couple of upstream fixes Benjamin Tissoires
3 siblings, 0 replies; 6+ messages in thread
From: Benjamin Tissoires @ 2024-01-24 11:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
Andrii Nakryiko
Cc: linux-input, linux-kernel, bpf, Benjamin Tissoires
Follow the docs at Documentation/bpf/kfuncs.rst:
- declare the function with `__bpf_kfunc`
- disables missing prototype warnings, which allows to remove them from
include/linux/hid-bpf.h
Removing the prototypes is not an issue because we currently have to
redeclare them when writing the BPF program. They will eventually be
generated by bpftool directly AFAIU.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
changes in v2:
* make use of __bpf_kfunc_start/end_defs() instead of manual push/pop
---
drivers/hid/bpf/hid_bpf_dispatch.c | 18 +++++++++++++-----
include/linux/hid_bpf.h | 11 -----------
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 7903c8638e81..470ae2c29c94 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -143,6 +143,9 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
}
EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
+/* Disables missing prototype warnings */
+__bpf_kfunc_start_defs();
+
/**
* hid_bpf_get_data - Get the kernel memory pointer associated with the context @ctx
*
@@ -152,7 +155,7 @@ EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
*
* @returns %NULL on error, an %__u8 memory pointer on success
*/
-noinline __u8 *
+__bpf_kfunc __u8 *
hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr_buf_size)
{
struct hid_bpf_ctx_kern *ctx_kern;
@@ -167,6 +170,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr
return ctx_kern->data + offset;
}
+__bpf_kfunc_end_defs();
/*
* The following set contains all functions we agree BPF programs
@@ -274,6 +278,9 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
return fd;
}
+/* Disables missing prototype warnings */
+__bpf_kfunc_start_defs();
+
/**
* hid_bpf_attach_prog - Attach the given @prog_fd to the given HID device
*
@@ -286,7 +293,7 @@ static int do_hid_bpf_attach_prog(struct hid_device *hdev, int prog_fd, struct b
* is pinned to the BPF file system).
*/
/* called from syscall */
-noinline int
+__bpf_kfunc int
hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
{
struct hid_device *hdev;
@@ -338,7 +345,7 @@ hid_bpf_attach_prog(unsigned int hid_id, int prog_fd, __u32 flags)
*
* @returns A pointer to &struct hid_bpf_ctx on success, %NULL on error.
*/
-noinline struct hid_bpf_ctx *
+__bpf_kfunc struct hid_bpf_ctx *
hid_bpf_allocate_context(unsigned int hid_id)
{
struct hid_device *hdev;
@@ -371,7 +378,7 @@ hid_bpf_allocate_context(unsigned int hid_id)
* @ctx: the HID-BPF context to release
*
*/
-noinline void
+__bpf_kfunc void
hid_bpf_release_context(struct hid_bpf_ctx *ctx)
{
struct hid_bpf_ctx_kern *ctx_kern;
@@ -397,7 +404,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx)
*
* @returns %0 on success, a negative error code otherwise.
*/
-noinline int
+__bpf_kfunc 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)
{
@@ -465,6 +472,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
kfree(dma_data);
return ret;
}
+__bpf_kfunc_end_defs();
/* our HID-BPF entrypoints */
BTF_SET8_START(hid_bpf_fmodret_ids)
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index 840cd254172d..7118ac28d468 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -77,17 +77,6 @@ enum hid_bpf_attach_flags {
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 */
-__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
*/
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/3] HID: bpf: couple of upstream fixes
2024-01-24 11:26 [PATCH v2 0/3] HID: bpf: couple of upstream fixes Benjamin Tissoires
` (2 preceding siblings ...)
2024-01-24 11:26 ` [PATCH v2 3/3] HID: bpf: use __bpf_kfunc instead of noinline Benjamin Tissoires
@ 2024-01-31 10:38 ` Benjamin Tissoires
3 siblings, 0 replies; 6+ messages in thread
From: Benjamin Tissoires @ 2024-01-31 10:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dan Carpenter, Daniel Borkmann,
Andrii Nakryiko, Benjamin Tissoires
Cc: linux-input, linux-kernel, bpf, stable
On Wed, 24 Jan 2024 12:26:56 +0100, Benjamin Tissoires wrote:
> This is the v2 of this series of HID-BPF fixes.
> I have forgotten to include a Fixes tag in the first patch
> and got a review from Andrii on patch 2.
>
> And this first patch made me realize that something was fishy
> in the refcount of the hid devices. I was not crashing the system
> even if I accessed the struct hid_device after hid_destroy_device()
> was called, which was suspicious to say the least. So after some
> debugging I found the culprit and realized that I had a pretty
> nice memleak as soon as one HID-BPF program was attached to a HID
> device.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.8/upstream-fixes), thanks!
[1/3] HID: bpf: remove double fdget()
https://git.kernel.org/hid/hid/c/7cdd2108903a
[2/3] HID: bpf: actually free hdev memory after attaching a HID-BPF program
https://git.kernel.org/hid/hid/c/89be8aa5b0ec
[3/3] HID: bpf: use __bpf_kfunc instead of noinline
https://git.kernel.org/hid/hid/c/764ad6b02777
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread