mbox series

[v2,0/3] HID: bpf: couple of upstream fixes

Message ID 20240124-b4-hid-bpf-fixes-v2-0-052520b1e5e6@kernel.org (mailing list archive)
Headers show
Series HID: bpf: couple of upstream fixes | expand

Message

Benjamin Tissoires Jan. 24, 2024, 11:26 a.m. UTC
Hi,

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.

The good thing though is that this ref count prevents a crash in
case a HID-BPF program attempts to access a removed HID device when
hid_bpf_allocate_context() has been called but not yet released.

Anyway, for reference, the cover letter of v1:

---

Hi,

these are a couple of fixes for hid-bpf. The first one should
probably go in ASAP, after the reviews, and the second one is nice
to have and doesn't hurt much.

Thanks Dan for finding out the issue with bpf_prog_get()

Cheers,
Benjamin

To: Jiri Kosina <jikos@kernel.org>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
To: Daniel Borkmann <daniel@iogearbox.net>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc:  <linux-input@vger.kernel.org>
Cc:  <linux-kernel@vger.kernel.org>
Cc:  <bpf@vger.kernel.org>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---
Changes in v2:
- add Fixes tags
- handled Andrii review (use of __bpf_kfunc_start/end_defs())
- new patch to fetch ref counting of struct hid_device
- Link to v1: https://lore.kernel.org/r/20240123-b4-hid-bpf-fixes-v1-0-aa1fac734377@kernel.org

---
Benjamin Tissoires (3):
      HID: bpf: remove double fdget()
      HID: bpf: actually free hdev memory after attaching a HID-BPF program
      HID: bpf: use __bpf_kfunc instead of noinline

 drivers/hid/bpf/hid_bpf_dispatch.c  | 101 ++++++++++++++++++++++++++----------
 drivers/hid/bpf/hid_bpf_dispatch.h  |   4 +-
 drivers/hid/bpf/hid_bpf_jmp_table.c |  39 +++++++-------
 include/linux/hid_bpf.h             |  11 ----
 4 files changed, 95 insertions(+), 60 deletions(-)
---
base-commit: fef018d8199661962b5fc0f0d1501caa54b2b533
change-id: 20240123-b4-hid-bpf-fixes-662908fe2234

Best regards,

Comments

Benjamin Tissoires Jan. 31, 2024, 10:38 a.m. UTC | #1
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,