[PATCH] Input: appletouch - fix use-after-free in work handler during disconnect

Haowen Tu posted 1 patch 3 weeks, 1 day ago
drivers/input/mouse/appletouch.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] Input: appletouch - fix use-after-free in work handler during disconnect
Posted by Haowen Tu 3 weeks, 1 day ago
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.

Signed-off-by: Haowen Tu <tuhaowen@uniontech.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);
-- 
2.20.1
Re: [PATCH] Input: appletouch - fix use-after-free in work handler during disconnect
Posted by Dmitry Torokhov 2 weeks, 3 days ago
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@uniontech.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