linux-kernel - Re: [PATCH] Input: psmouse - fix use-after-free bugs due to rescheduled delayed works

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <15ef9eb6.22570.19a6221252c.Coremail.duoming@zju.edu.cn>
Date: Sat, 8 Nov 2025 14:22:10 +0800 (GMT+08:00)
From: duoming@....edu.cn
To: "Dmitry Torokhov" <dmitry.torokhov@...il.com>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	kuba@...nel.org, alexander.deucher@....com, pali@...nel.org,
	hverkuil+cisco@...nel.org, akpm@...ux-foundation.org,
	andriy.shevchenko@...ux.intel.com, tglx@...utronix.de,
	mingo@...nel.org, Jonathan.Cameron@...wei.com
Subject: Re: [PATCH] Input: psmouse - fix use-after-free bugs due to
 rescheduled delayed works

On Fri, 7 Nov 2025 21:43:37 -0800, Dmitry Torokhov wrote:
> > The flush_workqueue() in psmouse_disconnect() only blocks and waits for
> > work items that were already queued to the workqueue prior to its
> > invocation. Any work items submitted after flush_workqueue() is called
> > are not included in the set of tasks that the flush operation awaits.
> > This means that after flush_workqueue() has finished executing, the
> > resync_work and dev3_register_work could be rescheduled again, resulting
> > in the following two use-after-free scenarios:
> > 
> > 1. The psmouse structure is deallocated in psmouse_disconnect(), while
> > resync_work remains active and attempts to dereference the already
> > freed psmouse in psmouse_resync().
> > 
> > CPU 0                   | CPU 1
> > psmouse_disconnect()    | psmouse_receive_byte()
> >                         |   if(psmouse->state == ...)
> >   psmouse_set_state()   |
> >   flush_workqueue()     |
> >                         |   psmouse_queue_work() //reschedule
> >   kfree(psmouse); //FREE|
> >                         | psmouse_resync()
> >                         |   psmouse = container_of(); //USE
> >                         |   psmouse-> //USE
> 
> Before flushing the workqueue we set psmouse state to PSMOUSE_CMD_MODE,
> but psmouse_queue_work() is only called from psmouse_receive_byte() if
> the mouse is PSMOUSE_ACTIVE. Therefore there is no chance that work will
> be scheduled while psmouse instance is being freed.

I believe a potential race condition still exists between 
psmouse_receive_byte() and psmouse_disconnect(). Specifically, 
if the condition check 'if (psmouse->state == PSMOUSE_ACTIVATED)' 
has already been passed, and then we set the psmouse state to 
PSMOUSE_CMD_MODE and flush the workqueue, it is still possible 
for psmouse_queue_work() to be scheduled after flush_workqueue().

Here is the problematic sequence:

CPU 0                   | CPU 1
psmouse_disconnect()    | psmouse_receive_byte()
                        |   if(psmouse->state == PSMOUSE_ACTIVATED && ...)
  psmouse_set_state()   |
  flush_workqueue()     |
                        |   psmouse_queue_work()
  kfree(psmouse); //FREE|
                        | psmouse_resync()
                        |   psmouse = container_of(); //USE
                        |   psmouse-> //USE

> For ALPS, the work is a "single shot", so will not get rescheduled.

I agree with you, the 'rescheduled' is not accurate. But the race condition
is still possible. The dev3_register_work is initialized through alps_reconnect()
and scheduled when receiving the first bare PS/2 packet from an external
device connected to the ALPS touchpad's PS/2 port. When the device
is disconnecting, the original code does not cancel the dev3_register_work.
This leads to a use-after-free scenario where the alps_data is
freed in alps_disconnect(), but the dev3_register_work remains active
and may attempt to access the freed alps_data in alps_register_bare_ps2_mouse().

A typical race condition is illustrated below:

CPU 0 (cleanup)         | CPU 1 (delayed work)
alps_disconnect()       |
  kfree(priv);          |
                        | alps_register_bare_ps2_mouse()
                        |   priv = container_of(...); //USE
                        |   // access priv->... (USE)

Fix this by ensuring that the delayed work is canceled in alps_disconnect()
using disable_delayed_work_sync().

> However I think that the changes are improvement to the code. Please
> split in 2 (for psmouse-base and alps separately) and drop mentions of
> UAF in psmouse but rather mention that disable_delayed_work_sync() is
> more robust and efficient. I think if we switch to it we should be able
> to get rid of kpsmoused workqueue and use default system workqueue.

Thank you for your suggestions, I will split this into two patches: 
one to fix the issue in psmouse-base, and another for the ALPS.



Best regards,
Duoming Zhou