Re: [PATCH v2 1/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] logi_dj_remove() cancels djrcv_dev->work before disabling hardware interrupts, allowing new work to be scheduled during teardown and causing a Use-After-Free.
--

--- Patch [1]: [PATCH v2 1/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices ---
commit 971b266887902a1a6dc0bffd56ec903cd5386ed8
Author: Erik Håkansson <erikhakan@gmail.com>
HID: logitech: add Bolt receiver support for Logitech HID++ devices

Add Logitech Bolt receiver support to the Logitech HID receiver and HID++
drivers. This allows Bolt-connected HID++ devices to expose battery
information through the kernel power_supply path.

Link: https://lore.kernel.org/linux-input/345f7347-30a8-4568-a7a6-c70f54a52a8d@mcld.eu/

> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 381e4dc5aba7a..112ea6a7b2a65 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c

[ ... ]

> @@ -1745,6 +1750,24 @@ static int logi_dj_hidpp_event(struct hid_device *hdev,
>  
>  	dj_dev = djrcv_dev->paired_dj_devices[device_index];
>  
> +	/*
> +	 * Bolt receivers send explicit unpair notifications as HID++ events;
> +	 * queue device removal when we receive one.
> +	 */
> +	if (djrcv_dev->type == recvr_type_bolt &&
> +	    hidpp_report->report_id == REPORT_ID_HIDPP_SHORT &&
> +	    hidpp_report->sub_id == REPORT_TYPE_NOTIF_DEVICE_UNPAIRED) {
> +		struct dj_workitem workitem = {
> +			.device_index = device_index,
> +			.type = WORKITEM_TYPE_UNPAIRED,
> +		};
> +
> +		kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem));
> +		schedule_work(&djrcv_dev->work);

[Severity: High]
This is a pre-existing issue, but can this sequence cause a use-after-free
during device removal?

Looking at logi_dj_remove() in drivers/hid/hid-logitech-dj.c, it cancels
the work queue before stopping the hardware:

drivers/hid/hid-logitech-dj.c:logi_dj_remove() {
	...
	cancel_work_sync(&djrcv_dev->work);

	hid_hw_close(hdev);
	hid_hw_stop(hdev);
	...
}

If a user unplugs the USB receiver, and a Bolt unpair notification arrives
right after cancel_work_sync() finishes but before hid_hw_stop() disables
hardware events, will this schedule_work() call queue a new work item on a
djrcv_dev structure that is about to be freed?

> +		spin_unlock_irqrestore(&djrcv_dev->lock, flags);
> +		return false;
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713201201.391538-2-erikhakan@gmail.com?part=1