| Message ID | 20250509113905.34947e78@meshulam.tesarici.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Use after free in dispatch_hid_bpf_output_report() | expand |
Hi Petr, On May 09 2025, Petr Tesařík wrote: > Hi all, > > after installing v6.15-rc5 on my laptop, I'm running into an invalid > pointer dereference in dispatch_hid_bpf_output_report() on suspend. I > added some debugging messages (see patch below), and I can see this > sequence of events: > > [ 1568.571776] [ T7420] PM: suspend entry (deep) > [ 1568.602245] [ T7420] Filesystems sync: 0.030 seconds > [ 1568.613183] [ T1704] hid-generic 0005:04F2:182A.0004: CLEANED UP srcu 00000000b7570e01 > [ 1568.613348] [ T724] hid-generic 0005:04F2:182A.0004: UAF srcu 00000000b7570e01 > [ 1568.616215] [ T7420] Freezing user space processes > > The HID device is a Bluetooth keyboard (using bluez 5.79), which > (presumably) gets disconnected on suspend. > > FTR I didn't encounter any such issues with v6.14. Thanks for the patch. I already cc-ed you to the other debugging thread[0], because I am slightly in favor of the other approach, based on ->destroyed. Also that other patch prevents the race in other hooks. Cheers, Benjamin [0] https://lore.kernel.org/linux-input/xyfdjeijtdt4sgb4zjmlibdbbvaaly3m3wiqhk7tu35cb2bpip@axziyhfcqx6w/T/#t > > Petr T > > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c > index 2e96ec6a3073..f284175e8b0b 100644 > --- a/drivers/hid/bpf/hid_bpf_dispatch.c > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c > @@ -130,6 +130,11 @@ int dispatch_hid_bpf_output_report(struct hid_device *hdev, > struct hid_bpf_ops *e; > int ret, idx; > > + if (unlikely(!hdev->bpf.srcu.sda)) { > + hid_warn(hdev, "UAF srcu %p", &hdev->bpf.srcu); > + return 0; > + } > + > 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)) { > @@ -143,6 +148,8 @@ int dispatch_hid_bpf_output_report(struct hid_device *hdev, > ret = 0; > > out: > + if (unlikely(!hdev->bpf.srcu.sda)) > + hid_warn(hdev, "RACE srcu %p", &hdev->bpf.srcu); > srcu_read_unlock(&hdev->bpf.srcu, idx); > return ret; > } > @@ -631,6 +638,7 @@ void hid_bpf_destroy_device(struct hid_device *hdev) > > synchronize_srcu(&hdev->bpf.srcu); > cleanup_srcu_struct(&hdev->bpf.srcu); > + hid_info(hdev, "CLEANED UP srcu %p", &hdev->bpf.srcu); > } > EXPORT_SYMBOL_GPL(hid_bpf_destroy_device); >
On Mon, 12 May 2025 16:47:12 +0200 Benjamin Tissoires <bentiss@kernel.org> wrote: > Hi Petr, > > > On May 09 2025, Petr Tesařík wrote: > > Hi all, > > > > after installing v6.15-rc5 on my laptop, I'm running into an invalid > > pointer dereference in dispatch_hid_bpf_output_report() on suspend. I > > added some debugging messages (see patch below), and I can see this > > sequence of events: > > > > [ 1568.571776] [ T7420] PM: suspend entry (deep) > > [ 1568.602245] [ T7420] Filesystems sync: 0.030 seconds > > [ 1568.613183] [ T1704] hid-generic 0005:04F2:182A.0004: CLEANED UP srcu 00000000b7570e01 > > [ 1568.613348] [ T724] hid-generic 0005:04F2:182A.0004: UAF srcu 00000000b7570e01 > > [ 1568.616215] [ T7420] Freezing user space processes > > > > The HID device is a Bluetooth keyboard (using bluez 5.79), which > > (presumably) gets disconnected on suspend. > > > > FTR I didn't encounter any such issues with v6.14. > > Thanks for the patch. I already cc-ed you to the other debugging > thread[0], because I am slightly in favor of the other approach, based > on ->destroyed. Also that other patch prevents the race in other hooks. Oh, sure thing. My patch was not intended as a fix, rather as a way to debug the issue, and I included it only for reference. Thank you for your quick fix! Petr T
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c index 2e96ec6a3073..f284175e8b0b 100644 --- a/drivers/hid/bpf/hid_bpf_dispatch.c +++ b/drivers/hid/bpf/hid_bpf_dispatch.c @@ -130,6 +130,11 @@ int dispatch_hid_bpf_output_report(struct hid_device *hdev, struct hid_bpf_ops *e; int ret, idx; + if (unlikely(!hdev->bpf.srcu.sda)) { + hid_warn(hdev, "UAF srcu %p", &hdev->bpf.srcu); + return 0; + } + 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)) { @@ -143,6 +148,8 @@ int dispatch_hid_bpf_output_report(struct hid_device *hdev, ret = 0; out: + if (unlikely(!hdev->bpf.srcu.sda)) + hid_warn(hdev, "RACE srcu %p", &hdev->bpf.srcu); srcu_read_unlock(&hdev->bpf.srcu, idx); return ret; } @@ -631,6 +638,7 @@ void hid_bpf_destroy_device(struct hid_device *hdev) synchronize_srcu(&hdev->bpf.srcu); cleanup_srcu_struct(&hdev->bpf.srcu); + hid_info(hdev, "CLEANED UP srcu %p", &hdev->bpf.srcu); } EXPORT_SYMBOL_GPL(hid_bpf_destroy_device);