lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ