[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <qg6meqq256s277awsoo4xulkdthfimvhlowp7bmuk2annscm6q@hqtttyufeonm>
Date: Tue, 20 Jan 2026 10:18:24 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Haowen Tu <tuhaowen@...ontech.com>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: appletouch - fix use-after-free in work handler
during disconnect
Hi Haowen,
On Fri, Jan 16, 2026 at 11:11:13AM +0800, Haowen Tu wrote:
> The atp_reinit work item is initialized in atp_probe() and scheduled
> from atp_complete() URB callback when the device becomes idle. During
> device disconnection, the current implementation calls usb_kill_urb()
> in atp_disconnect() but fails to prevent the work from being executed
> after the atp structure has been freed.
>
> Although usb_kill_urb() terminates the URB and its callbacks, there is
> a critical race window: if schedule_work() is called in atp_complete()
> just before usb_kill_urb() takes effect, the work item can still be
> queued. Since atp_disconnect() immediately proceeds to free the atp
> structure without canceling pending work, this leads to a use-after-free
> vulnerability when the work handler executes.
>
> The race condition:
>
> CPU 0 (disconnect path) | CPU 1 (URB completion)
> atp_disconnect() |
> usb_kill_urb(dev->urb) | atp_complete()
> | schedule_work(&dev->work)
> input_unregister_device() |
> usb_free_coherent() |
> usb_free_urb() |
> kfree(dev); // FREE | atp_reinit()
> | dev = container_of(...) // USE
> | atp_geyser_init(dev) // USE
> | dev->urb // USE
> | dev->intf // USE
>
> Fix this by adding disable_work_sync() in atp_disconnect() after
> usb_kill_urb() to ensure the work is properly canceled and cannot
> be rescheduled before the atp structure is freed.
I do not think there is actually a race. The urb is only
submitted/started when the device is open, the work in properly
cancelled in atp_close(), and the input core will ensure atp_close() is
called for opened devices. In fact, that usb_kill_urb() call in
atp_disconnect() is not needed at all.
I see there is a potential race handling dev->open in the driver, I just
posted a patch addressing it.
>
> Signed-off-by: Haowen Tu <tuhaowen@...ontech.com>
> ---
> drivers/input/mouse/appletouch.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index e669f86f1882..aa2870a87eee 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -946,6 +946,7 @@ static void atp_disconnect(struct usb_interface *iface)
> usb_set_intfdata(iface, NULL);
> if (dev) {
> usb_kill_urb(dev->urb);
> + disable_work_sync(&dev->work);
> input_unregister_device(dev->input);
> usb_free_coherent(dev->udev, dev->info->datalen,
> dev->data, dev->urb->transfer_dma);
Thanks.
--
Dmitry
Powered by blists - more mailing lists