HID: bpf: serialize device reference release in struct_ops destroy path

__hid_bpf_ops_destroy_device() and hid_bpf_unreg() can race on the
same registration reference, double-putting struct hid_device and
freeing it while hid_destroy_device() still uses it.  Serialize the
remove/NULL decision under hdev->bpf.prog_list_lock so exactly one
path releases each registration reference: unreg re-checks ops->hdev
under the lock and returns without putting when the destroy path
already cleared it; all put_device() calls happen after the lock is
dropped, which is safe because a concurrent unreg then observes
ops->hdev == NULL under the lock.

Background: each successful attach (hid_bpf_ops_reg) acquires one
device reference (hid_get_device()).  Two paths can release it:

- device destruction: hid_destroy_device() -> hid_bpf_destroy_device()
  -> __hid_bpf_ops_destroy_device(), which walks hdev->bpf.prog_list
  under rcu_read_lock() and drops one reference per attached program;
- BPF link release: bpf map delete (no BPF_F_LINK) synchronously calls
  st_ops->unreg() -> hid_bpf_unreg(), which drops the reference for
  its own registration.

The coordination handshake (e->hdev = NULL on the destroy side vs
"if (!hdev) return" on the unreg side) is a TOCTOU check: the two
paths run under different lock domains (rcu_read_lock vs
prog_list_lock), so a concurrent unreg can read ops->hdev as
non-NULL, block on prog_list_lock, and then proceed while the
destroy traversal executes - both paths then drop the same
reference.  The refcount reaches zero legitimately (each decrement
is individually valid), so no refcount_t saturation fires: the
device is simply freed while the transport is still inside
hid_destroy_device(), and subsequent teardown touches freed memory.

The fix serializes the remove/NULL decision under prog_list_lock on
both sides and moves the destroy-side puts outside the lock.  With
the lock held, plain reads/writes of ops->hdev are sufficient; no
READ_ONCE/WRITE_ONCE are added, keeping the patch minimal.

Unlocked-read safety: the unlocked read of ops->hdev at the top of
hid_bpf_unreg() cannot touch a freed device, because the unreg path
itself still holds this registration's reference (released only by
its own hid_put_device() after the lock is dropped), and a destroy
traversal that already cleared ops->hdev makes the lock-internal
re-check return early without any put.  At most one of the two
paths releases each registration reference.

Fixes: ebc0d8093e ("HID: bpf: implement HID-BPF through bpf_struct_ops")
Cc: stable@vger.kernel.org
Signed-off-by: Shen Yongchao <grayhat@foxmail.com>
Assisted-by: Hermes:kimi-k3
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
This commit is contained in:
Shen Yongchao 2026-08-03 22:31:57 +08:00 committed by Benjamin Tissoires
parent 4956993bb3
commit 9cdc7e6dc7

View File

@ -256,6 +256,11 @@ static void hid_bpf_unreg(void *kdata, struct bpf_link *link)
mutex_lock(&hdev->bpf.prog_list_lock);
if (!ops->hdev) {
mutex_unlock(&hdev->bpf.prog_list_lock);
return;
}
list_del_rcu(&ops->list);
synchronize_srcu(&hdev->bpf.srcu);
ops->hdev = NULL;
@ -316,13 +321,17 @@ static struct bpf_struct_ops bpf_hid_bpf_ops = {
void __hid_bpf_ops_destroy_device(struct hid_device *hdev)
{
struct hid_bpf_ops *e;
int count = 0;
rcu_read_lock();
list_for_each_entry_rcu(e, &hdev->bpf.prog_list, list) {
hid_put_device(hdev);
mutex_lock(&hdev->bpf.prog_list_lock);
list_for_each_entry(e, &hdev->bpf.prog_list, list) {
e->hdev = NULL;
count++;
}
rcu_read_unlock();
mutex_unlock(&hdev->bpf.prog_list_lock);
while (count--)
hid_put_device(hdev);
}
static int __init hid_bpf_struct_ops_init(void)