[PATCH] HID: bpf: fix lockless race on device_data allocation

Bill Wendling posted 1 patch 15 hours ago
drivers/hid/bpf/hid_bpf_dispatch.c | 38 +++++++++++++++++++-----------
1 file changed, 24 insertions(+), 14 deletions(-)
[PATCH] HID: bpf: fix lockless race on device_data allocation
Posted by Bill Wendling 15 hours ago
When attaching a hid_device_event BPF program to an already-connected
HID device, hid_bpf_reg() calls hid_bpf_allocate_event_data() while
holding only hdev->bpf.prog_list_lock. Concurrently, incoming HID
reports invoke dispatch_hid_bpf_device_event() while holding only
hdev->driver_input_lock.

Previously, __hid_bpf_allocate_data() assigned *data (device_data)
before *size (allocated_data) without release semantics, and
dispatch_hid_bpf_device_event() loaded hdev->bpf.allocated_data and
initialized ctx_kern before checking whether hdev->bpf.device_data was
non-NULL. A concurrent reader could therefore observe device_data != NULL
with allocated_data == 0, skipping the memset() clear and dropping the
report with -EINVAL (or dereferencing NULL if device_data was reloaded
for the NULL check).

Use WRITE_ONCE() and smp_store_release() in __hid_bpf_allocate_data() so
allocated_data and the zeroed buffer are visible before device_data becomes
non-NULL, and pair it with smp_load_acquire() in
dispatch_hid_bpf_device_event() before reading allocated_data and
initializing ctx_kern.

Fixes: 658ee5a64fcf ("HID: bpf: allocate data memory for device_event BPF programs")
Assisted-by: LLM
Signed-off-by: Bill Wendling <morbo@google.com>
---
 drivers/hid/bpf/hid_bpf_dispatch.c | 38 +++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index d46779b63660..1a0c4fa6ed86 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -28,16 +28,10 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
 			      size_t *buf_size, u32 *size, int interrupt, u64 source,
 			      bool from_bpf)
 {
-	struct hid_bpf_ctx_kern ctx_kern = {
-		.ctx = {
-			.hid = hdev,
-			.allocated_size = hdev->bpf.allocated_data,
-			.size = *size,
-		},
-		.data = hdev->bpf.device_data,
-		.from_bpf = from_bpf,
-	};
+	struct hid_bpf_ctx_kern ctx_kern;
 	struct hid_bpf_ops *e;
+	u8 *device_data;
+	u32 allocated_data;
 	int ret;
 
 	if (unlikely(hdev->bpf.destroyed))
@@ -46,11 +40,26 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
 	if (type >= HID_REPORT_TYPES)
 		return ERR_PTR(-EINVAL);
 
-	/* no program has been attached yet */
-	if (!hdev->bpf.device_data)
+	/*
+	 * No program has been attached yet. Pairs with smp_store_release()
+	 * in __hid_bpf_allocate_data().
+	 */
+	device_data = smp_load_acquire(&hdev->bpf.device_data);
+	if (!device_data)
 		return data;
 
-	memset(ctx_kern.data, 0, hdev->bpf.allocated_data);
+	allocated_data = READ_ONCE(hdev->bpf.allocated_data);
+	ctx_kern = (struct hid_bpf_ctx_kern){
+		.ctx = {
+			.hid = hdev,
+			.allocated_size = allocated_data,
+			.size = *size,
+		},
+		.data = device_data,
+		.from_bpf = from_bpf,
+	};
+
+	memset(ctx_kern.data, 0, allocated_data);
 	memcpy(ctx_kern.data, data, *size);
 
 	rcu_read_lock();
@@ -255,8 +264,9 @@ static int __hid_bpf_allocate_data(struct hid_device *hdev, u8 **data, u32 *size
 	if (!alloc_data)
 		return -ENOMEM;
 
-	*data = alloc_data;
-	*size = alloc_size;
+	WRITE_ONCE(*size, alloc_size);
+	/* Pairs with smp_load_acquire() in dispatch_hid_bpf_device_event() */
+	smp_store_release(data, alloc_data);
 
 	return 0;
 }
-- 
2.56.0.rc1.310.g51773c2048-goog