* [PATCH 0/2] HID: bpf: close device teardown races @ 2026-11-02 11:58 Yifan Zhang 2026-11-02 11:58 ` [PATCH 1/2] HID: bpf: serialize hardware operations with device destruction Yifan Zhang 2026-11-02 11:58 ` [PATCH 2/2] HID: bpf: reject struct_ops registration during " Yifan Zhang 0 siblings, 2 replies; 6+ messages in thread From: Yifan Zhang @ 2026-11-02 11:58 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, linux-input Cc: Greg Kroah-Hartman, Alexei Starovoitov, bpf, linux-kernel HID-BPF can race device destruction in two paths. A hardware request can outlive transport-private state, and a struct_ops registration can resume after destruction has cleaned the device's SRCU state. Serialize HID-BPF hardware operations with the destruction transition, then reject registrations that acquire prog_list_lock after that transition. Yifan Zhang (2): HID: bpf: serialize hardware operations with device destruction HID: bpf: reject struct_ops registration during device destruction drivers/hid/bpf/hid_bpf_dispatch.c | 36 +++++++++++++++++++--------- drivers/hid/bpf/hid_bpf_struct_ops.c | 5 ++++ 2 files changed, 30 insertions(+), 11 deletions(-) -- 2.39.5 (Apple Git-154) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] HID: bpf: serialize hardware operations with device destruction 2026-11-02 11:58 [PATCH 0/2] HID: bpf: close device teardown races Yifan Zhang @ 2026-11-02 11:58 ` Yifan Zhang 2026-11-02 12:15 ` sashiko-bot 2026-11-02 16:21 ` Alexei Starovoitov 2026-11-02 11:58 ` [PATCH 2/2] HID: bpf: reject struct_ops registration during " Yifan Zhang 1 sibling, 2 replies; 6+ messages in thread From: Yifan Zhang @ 2026-11-02 11:58 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, linux-input Cc: Greg Kroah-Hartman, Alexei Starovoitov, bpf, linux-kernel A HID-BPF syscall context pins struct hid_device, but it does not pin the transport-private data referenced by the low-level driver. A hardware request can therefore sleep in the UHID report-wait path while closing the UHID file destroys the HID device, wakes the waiter, and frees struct uhid_device. The awakened request then accesses the freed object. KASAN reports: BUG: KASAN: use-after-free in __uhid_report_queue_and_wait Read of size 1 uhid_hid_raw_request __hid_hw_raw_request hid_bpf_hw_request bpf_prog_test_run_syscall Hold prog_list_lock across HID-BPF hardware operations. Device destruction sets destroyed while holding the same lock, so it waits for an operation that has already started. Later operations return -ENODEV before reaching the low-level driver. Apply the same lifetime rule to output reports. The reproducer completed 100 close-versus-request trials without a KASAN report with this change. Fixes: 91a7f802d185 ("HID: bpf: introduce hid_hw_request()") Closes: https://lore.kernel.org/linux-input/179317460423.87432.1451672332362541157@stu.pku.edu.cn/ Signed-off-by: Yifan Zhang <yfzhang23@stu.pku.edu.cn> --- drivers/hid/bpf/hid_bpf_dispatch.c | 36 +++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c index 6b467c74601cb..4fea42dd2f392 100644 --- a/drivers/hid/bpf/hid_bpf_dispatch.c +++ b/drivers/hid/bpf/hid_bpf_dispatch.c @@ -444,14 +444,20 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, if (!dma_data) return -ENOMEM; - ret = hid_ops->hid_hw_raw_request(ctx->hid, - dma_data[0], - dma_data, - size, - rtype, - reqtype, - (u64)(long)ctx, - true); /* prevent infinite recursions */ + mutex_lock(&ctx->hid->bpf.prog_list_lock); + if (READ_ONCE(ctx->hid->bpf.destroyed)) { + ret = -ENODEV; + } else { + ret = hid_ops->hid_hw_raw_request(ctx->hid, + dma_data[0], + dma_data, + size, + rtype, + reqtype, + (u64)(long)ctx, + true); /* prevent infinite recursions */ + } + mutex_unlock(&ctx->hid->bpf.prog_list_lock); if (ret > (ssize_t)size) ret = size; @@ -492,7 +498,13 @@ hid_bpf_hw_output_report(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz) if (!dma_data) return -ENOMEM; - ret = hid_ops->hid_hw_output_report(ctx->hid, dma_data, size, (u64)(long)ctx, true); + mutex_lock(&ctx->hid->bpf.prog_list_lock); + if (READ_ONCE(ctx->hid->bpf.destroyed)) + ret = -ENODEV; + else + ret = hid_ops->hid_hw_output_report(ctx->hid, dma_data, size, + (u64)(long)ctx, true); + mutex_unlock(&ctx->hid->bpf.prog_list_lock); kfree(dma_data); return ret; @@ -641,8 +653,10 @@ void hid_bpf_destroy_device(struct hid_device *hdev) if (!hdev) return; - /* mark the device as destroyed in bpf so we don't reattach it */ - hdev->bpf.destroyed = true; + /* Block new BPF hardware operations before transport teardown. */ + mutex_lock(&hdev->bpf.prog_list_lock); + WRITE_ONCE(hdev->bpf.destroyed, true); + mutex_unlock(&hdev->bpf.prog_list_lock); __hid_bpf_ops_destroy_device(hdev); -- 2.39.5 (Apple Git-154) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] HID: bpf: serialize hardware operations with device destruction 2026-11-02 11:58 ` [PATCH 1/2] HID: bpf: serialize hardware operations with device destruction Yifan Zhang @ 2026-11-02 12:15 ` sashiko-bot 2026-11-02 16:21 ` Alexei Starovoitov 1 sibling, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-11-02 12:15 UTC (permalink / raw) To: Yifan Zhang, linux-input; +Cc: bpf, dmitry.torokhov Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Critical] HID: bpf: report UAF in __hid_bpf_hw_check_params() during device teardown - [High] HID: bpf: ABBA deadlock between prog_list_lock and srcu in hw request kfuncs - [High] HID: bpf: recursive prog_list_lock deadlock in hid_bpf_hw_request() - [High] HID: bpf: lock inversion between prog_list_lock and driver_input_lock -- commit 49a737ce63da80cb915d4530a8a28ce52af759cc Author: Yifan Zhang <yfzhang23@stu.pku.edu.cn> HID: bpf: serialize hardware operations with device destruction This commit serializes HID-BPF hardware requests and output reports with device destruction by holding prog_list_lock and checking the destroyed flag across low-level driver calls in hid_bpf_hw_request() and hid_bpf_hw_output_report(). > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c > index 6b467c74601cb..4fea42dd2f392 100644 > --- a/drivers/hid/bpf/hid_bpf_dispatch.c > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c > @@ -444,14 +444,20 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, > if (!dma_data) > return -ENOMEM; > > - ret = hid_ops->hid_hw_raw_request(ctx->hid, > - dma_data[0], > - dma_data, > - size, > - rtype, > - reqtype, > - (u64)(long)ctx, > - true); /* prevent infinite recursions */ > + mutex_lock(&ctx->hid->bpf.prog_list_lock); > + if (READ_ONCE(ctx->hid->bpf.destroyed)) { > + ret = -ENODEV; > + } else { > + ret = hid_ops->hid_hw_raw_request(ctx->hid, > + dma_data[0], > + dma_data, > + size, > + rtype, > + reqtype, > + (u64)(long)ctx, > + true); /* prevent infinite recursions */ > + } > + mutex_unlock(&ctx->hid->bpf.prog_list_lock); [Severity: High] Can acquiring ctx->hid->bpf.prog_list_lock here in hid_bpf_hw_request() (and below in hid_bpf_hw_output_report()) in drivers/hid/bpf/hid_bpf_dispatch.c cause an ABBA deadlock with hdev->bpf.srcu? In drivers/hid/bpf/hid_bpf_struct_ops.c, both hid_bpf_reg() and hid_bpf_unreg() hold prog_list_lock while waiting on synchronize_srcu(): drivers/hid/bpf/hid_bpf_struct_ops.c:hid_bpf_unreg() { ... mutex_lock(&hdev->bpf.prog_list_lock); ... list_del_rcu(&ops->list); synchronize_srcu(&hdev->bpf.srcu); ... mutex_unlock(&hdev->bpf.prog_list_lock); } When hid_bpf_hw_request() or hid_bpf_hw_output_report() is invoked from a syscall BPF program, it also takes prog_list_lock and then calls __hid_hw_raw_request() or __hid_hw_output_report(), which enters dispatch_hid_bpf_raw_requests() or dispatch_hid_bpf_output_report() and takes srcu_read_lock(&hdev->bpf.srcu). Both paths establish the lock order prog_list_lock -> hdev->bpf.srcu. However, hid_bpf_hw_request() and hid_bpf_hw_output_report() are also registered in hid_bpf_kfunc_ids for BPF_PROG_TYPE_STRUCT_OPS, so sleepable struct_ops callbacks (e->hid_hw_request and e->hid_hw_output_report) can call them directly. When a non-BPF caller such as hidraw invokes __hid_hw_raw_request(..., from_bpf=false) or __hid_hw_output_report(..., from_bpf=false), dispatch_hid_bpf_raw_requests() or dispatch_hid_bpf_output_report() holds srcu_read_lock(&hdev->bpf.srcu) across the callback: drivers/hid/bpf/hid_bpf_dispatch.c:dispatch_hid_bpf_raw_requests() { ... idx = srcu_read_lock(&hdev->bpf.srcu); list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list, srcu_read_lock_held(&hdev->bpf.srcu)) { if (!e->hid_hw_request) continue; ret = e->hid_hw_request(&ctx_kern.ctx, reportnum, rtype, reqtype, source); if (ret) goto out; } ret = 0; out: srcu_read_unlock(&hdev->bpf.srcu, idx); return ret; } When that struct_ops callback calls hid_bpf_hw_request() or hid_bpf_hw_output_report(), it acquires prog_list_lock while holding srcu_read_lock(&hdev->bpf.srcu), inverting the lock order to hdev->bpf.srcu -> prog_list_lock and deadlocking if another thread is in hid_bpf_reg() or hid_bpf_unreg(). [Severity: High] Could holding ctx->hid->bpf.prog_list_lock across hid_ops->hid_hw_raw_request() in hid_bpf_hw_request() (and across hid_ops->hid_hw_output_report() in hid_bpf_hw_output_report()) also lead to a recursive mutex self-deadlock on the same task? When hid_ops->hid_hw_raw_request(..., true) or hid_ops->hid_hw_output_report(..., true) runs with prog_list_lock held, __hid_hw_raw_request() and __hid_hw_output_report() still invoke dispatch_hid_bpf_raw_requests() and dispatch_hid_bpf_output_report(), which call any attached sleepable struct_ops e->hid_hw_request or e->hid_hw_output_report programs on that hid_device. The recursion check at the start of hid_bpf_hw_request() and hid_bpf_hw_output_report() only inspects ctx_kern->from_bpf on the passed context: drivers/hid/bpf/hid_bpf_dispatch.c:hid_bpf_hw_request() { ctx_kern = container_of(ctx, struct hid_bpf_ctx_kern, ctx); if (ctx_kern->from_bpf) return -EDEADLOCK; ... } Because hid_bpf_allocate_context() is exposed in hid_bpf_kfunc_ids to sleepable struct_ops programs and returns a fresh context with from_bpf == false, wouldn't a struct_ops program that calls hid_bpf_allocate_context(hctx->hid->id) and passes the new context to hid_bpf_hw_request() or hid_bpf_hw_output_report() bypass the ctx_kern->from_bpf check and try to lock ctx->hid->bpf.prog_list_lock a second time? [Severity: High] Does holding ctx->hid->bpf.prog_list_lock across hid_ops->hid_hw_raw_request() and hid_ops->hid_hw_output_report() in drivers/hid/bpf/hid_bpf_dispatch.c introduce a lock inversion with hdev->driver_input_lock? In drivers/hid/hid-core.c, hid_device_probe() and hid_device_remove() acquire hdev->driver_input_lock before calling __hid_device_probe() (which runs call_hid_bpf_rdesc_fixup() and hdrv->probe()) and hdrv->remove(). If a sleepable struct_ops callback (such as hid_rdesc_fixup, hid_hw_request, or hid_hw_output_report) invoked under driver_input_lock calls hid_bpf_hw_request() or hid_bpf_hw_output_report(), it acquires prog_list_lock while holding driver_input_lock (driver_input_lock -> prog_list_lock). Conversely, while hid_bpf_hw_request() or hid_bpf_hw_output_report() holds prog_list_lock across hid_ops->hid_hw_raw_request() or hid_ops->hid_hw_output_report(), an attached sleepable struct_ops callback e->hid_hw_request or e->hid_hw_output_report can call hid_bpf_input_report(), which is exposed in hid_bpf_kfunc_ids and acquires driver_input_lock before checking ctx_kern->from_bpf: drivers/hid/bpf/hid_bpf_dispatch.c:hid_bpf_input_report() { ... ret = down_interruptible(&ctx->hid->driver_input_lock); if (ret) return ret; ... } Wouldn't that establish the reverse ordering prog_list_lock -> driver_input_lock? > > if (ret > (ssize_t)size) > ret = size; > @@ -492,7 +498,13 @@ hid_bpf_hw_output_report(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz) > if (!dma_data) > return -ENOMEM; > > - ret = hid_ops->hid_hw_output_report(ctx->hid, dma_data, size, (u64)(long)ctx, true); > + mutex_lock(&ctx->hid->bpf.prog_list_lock); > + if (READ_ONCE(ctx->hid->bpf.destroyed)) > + ret = -ENODEV; > + else > + ret = hid_ops->hid_hw_output_report(ctx->hid, dma_data, size, > + (u64)(long)ctx, true); > + mutex_unlock(&ctx->hid->bpf.prog_list_lock); > > kfree(dma_data); > return ret; > @@ -641,8 +653,10 @@ void hid_bpf_destroy_device(struct hid_device *hdev) > if (!hdev) > return; > > - /* mark the device as destroyed in bpf so we don't reattach it */ > - hdev->bpf.destroyed = true; > + /* Block new BPF hardware operations before transport teardown. */ > + mutex_lock(&hdev->bpf.prog_list_lock); > + WRITE_ONCE(hdev->bpf.destroyed, true); > + mutex_unlock(&hdev->bpf.prog_list_lock); [Severity: Critical] Is checking ctx->hid->bpf.destroyed after __hid_bpf_hw_check_params() in hid_bpf_hw_request() and hid_bpf_hw_output_report() sufficient to prevent a use-after-free on struct hid_report during device teardown? In drivers/hid/bpf/hid_bpf_dispatch.c, both hid_bpf_hw_request() and hid_bpf_hw_output_report() call __hid_bpf_hw_check_params() before acquiring ctx->hid->bpf.prog_list_lock and before checking ctx->hid->bpf.destroyed: drivers/hid/bpf/hid_bpf_dispatch.c:hid_bpf_hw_request() { ... /* check arguments */ ret = __hid_bpf_hw_check_params(ctx, buf, &size, rtype, true); if (ret) return ret; ... mutex_lock(&ctx->hid->bpf.prog_list_lock); if (READ_ONCE(ctx->hid->bpf.destroyed)) { ... } __hid_bpf_hw_check_params() looks up and dereferences struct hid_report from ctx->hid->report_enum: drivers/hid/bpf/hid_bpf_dispatch.c:__hid_bpf_hw_check_params() { ... report_enum = ctx->hid->report_enum + rtype; report = hid_ops->hid_get_report(report_enum, buf); if (!report) return -EINVAL; report_len = hid_report_len(report); ... } During hid_destroy_device() in drivers/hid/hid-core.c, hid_bpf_destroy_device() sets hdev->bpf.destroyed = true and unlocks prog_list_lock, and then hid_remove_device() -> device_del() -> hid_device_remove() -> hid_close_report() frees each report in report_enum->report_id_hash[] before zeroing report_enum: drivers/hid/hid-core.c:hid_close_report() { ... for (i = 0; i < HID_REPORT_TYPES; i++) { struct hid_report_enum *report_enum = device->report_enum + i; for (j = 0; j < HID_MAX_IDS; j++) { struct hid_report *report = report_enum->report_id_hash[j]; if (report) hid_free_report(report); } memset(report_enum, 0, sizeof(*report_enum)); } } Because a syscall BPF context allocated by hid_bpf_allocate_context() only pins struct hid_device via get_device(&hdev->dev), can a concurrent call to hid_bpf_hw_request() or hid_bpf_hw_output_report() race with hid_close_report() inside __hid_bpf_hw_check_params() and dereference a freed struct hid_report before reaching the prog_list_lock and destroyed check? -- Sashiko AI review · https://sashiko.dev/#/patchset/179362071770.29269.16022650793604369465@stu.pku.edu.cn?part=1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] HID: bpf: serialize hardware operations with device destruction 2026-11-02 11:58 ` [PATCH 1/2] HID: bpf: serialize hardware operations with device destruction Yifan Zhang 2026-11-02 12:15 ` sashiko-bot @ 2026-11-02 16:21 ` Alexei Starovoitov 1 sibling, 0 replies; 6+ messages in thread From: Alexei Starovoitov @ 2026-11-02 16:21 UTC (permalink / raw) To: Yifan Zhang, Jiri Kosina, Benjamin Tissoires, linux-input Cc: Greg Kroah-Hartman, bpf, linux-kernel On Mon, Nov 02, 2026 at 03:58 AM Yifan Zhang <yfzhang23@stu.pku.edu.cn> wrote: > Hold prog_list_lock across HID-BPF hardware operations. Device destruction > sets destroyed while holding the same lock, so it waits for an operation > that has already started. Later operations return -ENODEV before reaching > the low-level driver. Apply the same lifetime rule to output reports. sashiko is right. This will deadlock. pw-bot: cr ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] HID: bpf: reject struct_ops registration during device destruction 2026-11-02 11:58 [PATCH 0/2] HID: bpf: close device teardown races Yifan Zhang 2026-11-02 11:58 ` [PATCH 1/2] HID: bpf: serialize hardware operations with device destruction Yifan Zhang @ 2026-11-02 11:58 ` Yifan Zhang 2026-11-02 12:13 ` sashiko-bot 1 sibling, 1 reply; 6+ messages in thread From: Yifan Zhang @ 2026-11-02 11:58 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, linux-input Cc: Greg Kroah-Hartman, Alexei Starovoitov, bpf, linux-kernel hid_bpf_reg() can obtain a device reference before hid_destroy_device() removes the device from the bus, then block on prog_list_lock while HID-BPF destruction cleans the device's SRCU state. The registration path does not check the destroyed flag after taking the lock, so it can publish a new struct_ops entry and call synchronize_srcu() on the cleaned SRCU object. KASAN reports: BUG: KASAN: slab-use-after-free in check_init_srcu_struct Read of size 8 check_init_srcu_struct synchronize_srcu hid_bpf_reg bpf_struct_ops_link_create Check destroyed while holding prog_list_lock. The preceding teardown serialization change sets the flag while holding the same lock, so either a registration completes before destruction starts or it fails with -ENODEV without accessing SRCU. The reproducer no longer triggers a KASAN report with this change. Fixes: ebc0d8093e8c ("HID: bpf: implement HID-BPF through bpf_struct_ops") Closes: https://lore.kernel.org/linux-input/179317460937.87455.1799017093083467190@stu.pku.edu.cn/ Signed-off-by: Yifan Zhang <yfzhang23@stu.pku.edu.cn> --- drivers/hid/bpf/hid_bpf_struct_ops.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c index c90b68956cb3b..4c433f419c85e 100644 --- a/drivers/hid/bpf/hid_bpf_struct_ops.c +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c @@ -201,6 +201,11 @@ static int hid_bpf_reg(void *kdata, struct bpf_link *link) mutex_lock(&hdev->bpf.prog_list_lock); + if (READ_ONCE(hdev->bpf.destroyed)) { + err = -ENODEV; + goto out_unlock; + } + count = list_count_nodes(&hdev->bpf.prog_list); if (count >= HID_BPF_MAX_PROGS_PER_DEV) { err = -E2BIG; -- 2.39.5 (Apple Git-154) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] HID: bpf: reject struct_ops registration during device destruction 2026-11-02 11:58 ` [PATCH 2/2] HID: bpf: reject struct_ops registration during " Yifan Zhang @ 2026-11-02 12:13 ` sashiko-bot 0 siblings, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-11-02 12:13 UTC (permalink / raw) To: Yifan Zhang, linux-input; +Cc: bpf, dmitry.torokhov Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] HID: bpf: dangling ops->hdev pointer on error path in hid_bpf_reg() - [High] HID: bpf: deadlock on prog_list_lock and srcu in hid_bpf_hw_request() -- commit 318c71f825ad280448e4c11859ac62903f968c18 Author: Yifan Zhang <yfzhang23@stu.pku.edu.cn> HID: bpf: reject struct_ops registration during device destruction This series serializes HID-BPF hardware requests and output reports against device teardown using prog_list_lock and the destroyed flag, and rejects new struct_ops registrations once device destruction has started. > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c > index 6b467c74601cb..4fea42dd2f392 100644 > --- a/drivers/hid/bpf/hid_bpf_dispatch.c > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c > @@ -444,14 +444,20 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, > if (!dma_data) > return -ENOMEM; > > - ret = hid_ops->hid_hw_raw_request(ctx->hid, > - dma_data[0], > - dma_data, > - size, > - rtype, > - reqtype, > - (u64)(long)ctx, > - true); /* prevent infinite recursions */ > + mutex_lock(&ctx->hid->bpf.prog_list_lock); > + if (READ_ONCE(ctx->hid->bpf.destroyed)) { > + ret = -ENODEV; > + } else { > + ret = hid_ops->hid_hw_raw_request(ctx->hid, > + dma_data[0], > + dma_data, > + size, > + rtype, > + reqtype, > + (u64)(long)ctx, > + true); /* prevent infinite recursions */ > + } > + mutex_unlock(&ctx->hid->bpf.prog_list_lock); > > if (ret > (ssize_t)size) > ret = size; > @@ -492,7 +498,13 @@ hid_bpf_hw_output_report(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz) > if (!dma_data) > return -ENOMEM; > > - ret = hid_ops->hid_hw_output_report(ctx->hid, dma_data, size, (u64)(long)ctx, true); > + mutex_lock(&ctx->hid->bpf.prog_list_lock); > + if (READ_ONCE(ctx->hid->bpf.destroyed)) > + ret = -ENODEV; > + else > + ret = hid_ops->hid_hw_output_report(ctx->hid, dma_data, size, > + (u64)(long)ctx, true); > + mutex_unlock(&ctx->hid->bpf.prog_list_lock); [Severity: High] Can taking prog_list_lock in hid_bpf_hw_request() and hid_bpf_hw_output_report() deadlock against hdev->bpf.srcu or against itself? In drivers/hid/bpf/hid_bpf_struct_ops.c, hid_bpf_reg() and hid_bpf_unreg() acquire prog_list_lock and then wait in synchronize_srcu(): drivers/hid/bpf/hid_bpf_struct_ops.c:hid_bpf_reg() { mutex_lock(&hdev->bpf.prog_list_lock); ... synchronize_srcu(&hdev->bpf.srcu); } Meanwhile, in drivers/hid/bpf/hid_bpf_dispatch.c, dispatch_hid_bpf_raw_requests() and dispatch_hid_bpf_output_report() hold srcu_read_lock(&hdev->bpf.srcu) while invoking sleepable struct_ops programs (e->hid_hw_request and e->hid_hw_output_report): drivers/hid/bpf/hid_bpf_dispatch.c:dispatch_hid_bpf_raw_requests() { idx = srcu_read_lock(&hdev->bpf.srcu); list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list, srcu_read_lock_held(&hdev->bpf.srcu)) { if (!e->hid_hw_request) continue; ret = e->hid_hw_request(&ctx_kern.ctx, reportnum, rtype, reqtype, source); ... } srcu_read_unlock(&hdev->bpf.srcu, idx); } Those sleepable callbacks are permitted to call the kfuncs hid_bpf_hw_request() and hid_bpf_hw_output_report() (as done in tools/testing/selftests/hid/progs/hid.c). With prog_list_lock taken inside those kfuncs, a task running in dispatch_hid_bpf_raw_requests() or dispatch_hid_bpf_output_report() will lock srcu -> prog_list_lock, while a concurrent hid_bpf_reg() or hid_bpf_unreg() locks prog_list_lock -> srcu, causing an ABBA deadlock. Additionally, because prog_list_lock is held across hid_ops->hid_hw_raw_request() and hid_ops->hid_hw_output_report(), which invoke dispatch_hid_bpf_raw_requests() and dispatch_hid_bpf_output_report() on the same task, if an attached sleepable struct_ops program allocates a fresh context via hid_bpf_allocate_context() (where from_bpf is false) and calls hid_bpf_hw_request() or hid_bpf_hw_output_report() on the same device, the task will attempt to acquire prog_list_lock a second time and self-deadlock. [ ... ] > diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c > index c90b68956cb3b..4c433f419c85e 100644 > --- a/drivers/hid/bpf/hid_bpf_struct_ops.c > +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c > @@ -201,6 +201,11 @@ static int hid_bpf_reg(void *kdata, struct bpf_link *link) > > mutex_lock(&hdev->bpf.prog_list_lock); > > + if (READ_ONCE(hdev->bpf.destroyed)) { > + err = -ENODEV; > + goto out_unlock; > + } [Severity: Critical] Does jumping to out_unlock on error in hid_bpf_reg() leave a dangling pointer in ops->hdev after hid_put_device(hdev) is called? In drivers/hid/bpf/hid_bpf_struct_ops.c, hid_bpf_reg() assigns ops->hdev = hdev before acquiring prog_list_lock, and the error path under out_unlock drops the hdev reference without clearing ops->hdev: drivers/hid/bpf/hid_bpf_struct_ops.c:hid_bpf_reg() { hdev = hid_get_device(ops->hid_id); if (IS_ERR(hdev)) return PTR_ERR(hdev); ops->hdev = hdev; mutex_lock(&hdev->bpf.prog_list_lock); if (READ_ONCE(hdev->bpf.destroyed)) { err = -ENODEV; goto out_unlock; } ... out_unlock: mutex_unlock(&hdev->bpf.prog_list_lock); if (err) { if (hdev->bpf.rdesc_ops == ops) hdev->bpf.rdesc_ops = NULL; hid_put_device(hdev); } } When bpf_struct_ops_link_create() fails in st_ops->reg(), the struct_ops map's kdata is not reset, so leaving ops->hdev non-NULL causes any subsequent attach attempt on the map to fail at if (ops->hdev) return -EINVAL. More importantly, if the map is already attached via an active bpf_link while hid_destroy_device(hdev) runs, hid_bpf_destroy_device() calls __hid_bpf_ops_destroy_device(hdev) (which clears ops->hdev = NULL and drops the active link's hdev reference) before hid_remove_device(hdev) removes the device from hid_bus_type. If a second bpf_struct_ops_link_create() on the same map races in this window, it passes if (ops->hdev), gets a temporary reference via hid_get_device(), sets ops->hdev = hdev, hits the new hdev->bpf.destroyed check, and drops the temporary reference via hid_put_device(hdev) while leaving ops->hdev pointing to hdev. Once hid_destroy_device() frees hdev, closing the first bpf_link calls hid_bpf_unreg(), which sees ops->hdev != NULL and dereferences the freed hdev. Should ops->hdev be cleared in the if (err) block (or assigned only after the error checks succeed)? -- Sashiko AI review · https://sashiko.dev/#/patchset/179362071770.29269.16022650793604369465@stu.pku.edu.cn?part=2 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-02 16:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-02 11:58 [PATCH 0/2] HID: bpf: close device teardown races Yifan Zhang 2026-11-02 11:58 ` [PATCH 1/2] HID: bpf: serialize hardware operations with device destruction Yifan Zhang 2026-11-02 12:15 ` sashiko-bot 2026-11-02 16:21 ` Alexei Starovoitov 2026-11-02 11:58 ` [PATCH 2/2] HID: bpf: reject struct_ops registration during " Yifan Zhang 2026-11-02 12:13 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.