[PATCH] Input: byd - synchronize timer deletion before freeing private data

[PATCH] Input: byd - synchronize timer deletion before freeing private data

[Linmao Li]

byd_disconnect() uses timer_delete() before freeing the driver's private
data.  This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer.  A callback racing
with disconnect can therefore access the private data after it has been
freed.

Use timer_delete_sync() to wait for any running callback before releasing
the private data.

Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/input/mouse/byd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
index f5770a3af2f1..7a69724c1ac6 100644
--- a/drivers/input/mouse/byd.c
+++ b/drivers/input/mouse/byd.c
@@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
 	struct byd_data *priv = psmouse->private;
 
 	if (priv) {
-		timer_delete(&priv->timer);
+		timer_delete_sync(&priv->timer);
 		kfree(psmouse->private);
 		psmouse->private = NULL;
 	}
-- 
2.25.1

Re: [PATCH] Input: byd - synchronize timer deletion before freeing private data

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free or NULL pointer dereference in byd_process_byte() due to missing RX synchronization during disconnect.
- [High] Improper timer teardown API allows the timer to be re-armed after sync, causing a delayed Use-After-Free.
--

--- Patch [1]: [PATCH] Input: byd - synchronize timer deletion before freeing private data ---
commit eb1fe8255d54e5e900b504d5aed98e3fff1721fb
Author: Linmao Li <lilinmao@kylinos.cn>

Input: byd - synchronize timer deletion before freeing private data
    
byd_disconnect() uses timer_delete() before freeing the driver's private
data.  This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer.  A callback racing
with disconnect can therefore access the private data after it has been
freed.
    
Use timer_delete_sync() to wait for any running callback before releasing
the private data.
    
Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>

> diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
> index f5770a3af2f12..7a69724c1ac6b 100644
> --- a/drivers/input/mouse/byd.c
> +++ b/drivers/input/mouse/byd.c
> @@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
>  	struct byd_data *priv = psmouse->private;
>  
>  	if (priv) {
> -		timer_delete(&priv->timer);
> +		timer_delete_sync(&priv->timer);

[Severity: High]
This isn't a bug introduced by this patch, but does timer_delete_sync()
prevent the timer from being re-armed concurrently?

Because device interrupts are not disabled during byd_disconnect(),
byd_process_byte() can execute immediately after timer_delete_sync()
returns but before psmouse->private is freed.

If byd_process_byte() processes a touch event, it can call mod_timer()
and successfully re-arm the timer on the priv structure:

drivers/input/mouse/byd.c:byd_process_byte() {
	...
	mod_timer(&priv->timer, jiffies + BYD_TOUCH_TIMEOUT);
	...
}

When byd_disconnect() subsequently frees priv, won't the timer callback
execute on the freed memory?

Should this use timer_shutdown_sync() instead to permanently prevent the timer
from being re-armed during teardown?

>  		kfree(psmouse->private);
>  		psmouse->private = NULL;

[Severity: High]
This is a pre-existing issue, but is it possible for an interrupt to trigger
byd_process_byte() concurrently with or immediately after freeing this memory?

The core psmouse_disconnect() function invokes the driver's disconnect callback
without first stopping device interrupts (such as by using serio_pause_rx()).

If the hardware sends a byte during device teardown, ps2_interrupt() can
route the data to the active protocol handler byd_process_byte(). This
unconditionally dereferences the freed or NULL psmouse->private pointer:

drivers/input/mouse/byd.c:byd_process_byte() {
	struct byd_data *priv = psmouse->private;
	...
}

Does the receive path need to be paused or synchronized before freeing the
private data to prevent a use-after-free or NULL pointer dereference?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720033030.1580891-1-lilinmao@kylinos.cn?part=1

Re: [PATCH] Input: byd - synchronize timer deletion before freeing private data

[Dmitry Torokhov]

On Mon, Jul 20, 2026 at 03:48:54AM +0000, sashiko-bot@kernel.org wrote:
> 
> Should this use timer_shutdown_sync() instead to permanently prevent the timer
> from being re-armed during teardown?

This is a good suggestion.

> 
> >  		kfree(psmouse->private);
> >  		psmouse->private = NULL;
> 
> [Severity: High]
> This is a pre-existing issue, but is it possible for an interrupt to trigger
> byd_process_byte() concurrently with or immediately after freeing this memory?

This has to be solved in psmouse core, I'll take a look.

Thanks.

-- 
Dmitry

[PATCH v2] Input: byd - synchronize timer deletion before freeing private data

[Linmao Li]

byd_disconnect() uses timer_delete() before freeing the driver's private
data.  This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer.  A callback racing
with disconnect can therefore access the private data after it has been
freed.  The timer can also still be re-armed by byd_process_byte() while
the disconnect is in progress.

Use timer_shutdown_sync() before freeing the private data: it waits for
a running callback and turns any later re-arm attempt into a no-op.

Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
v2:
- use timer_shutdown_sync() instead of timer_delete_sync() so the
  timer cannot be re-armed by byd_process_byte() while disconnect is
  in progress (Sashiko review, Dmitry)

 drivers/input/mouse/byd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
index f5770a3af2f1..5fc3c629590a 100644
--- a/drivers/input/mouse/byd.c
+++ b/drivers/input/mouse/byd.c
@@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
 	struct byd_data *priv = psmouse->private;
 
 	if (priv) {
-		timer_delete(&priv->timer);
+		timer_shutdown_sync(&priv->timer);
 		kfree(psmouse->private);
 		psmouse->private = NULL;
 	}
-- 
2.25.1

Re: [PATCH v2] Input: byd - synchronize timer deletion before freeing private data

[sashiko-bot]

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

Pre-existing issues:
- [High] Concurrent execution of the protocol handler `byd_process_byte()` during or after `byd_disconnect()` can lead to a Use-After-Free or NULL pointer dereference of the driver's private data.
--

--- Patch [1]: [PATCH v2] Input: byd - synchronize timer deletion before freeing private data ---
commit e08a94a088b6cedd4010692fa9ab4cd3e5287e71
Author: Linmao Li <lilinmao@kylinos.cn>

    Input: byd - synchronize timer deletion before freeing private data
    
    byd_disconnect() uses timer_delete() before freeing the driver's private
    data.  This does not wait for a running byd_clear_touch() callback, which
    dereferences the private data and its psmouse pointer.  A callback racing
    with disconnect can therefore access the private data after it has been
    freed.  The timer can also still be re-armed by byd_process_byte() while
    the disconnect is in progress.
    
    Use timer_shutdown_sync() before freeing the private data: it waits for
    a running callback and turns any later re-arm attempt into a no-op.

> diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
> index f5770a3af2f12..5fc3c629590ae 100644
> --- a/drivers/input/mouse/byd.c
> +++ b/drivers/input/mouse/byd.c
> @@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
>  	struct byd_data *priv = psmouse->private;
>  
>  	if (priv) {
> -		timer_delete(&priv->timer);
> +		timer_shutdown_sync(&priv->timer);
>  		kfree(psmouse->private);
                ^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
This is a pre-existing issue, but could this free lead to a use-after-free
or null pointer dereference if a hardware interrupt occurs concurrently?

During disconnect, the psmouse core sets the software state to
PSMOUSE_CMD_MODE in psmouse_disconnect(), which still allows the interrupt
handler to process incoming bytes.

If a full packet is received, the protocol handler byd_process_byte() is
called and unconditionally dereferences psmouse->private, which may have
just been freed or set to NULL:

drivers/input/mouse/byd.c:byd_process_byte() {
    struct byd_data *priv = psmouse->private;
    ...
    switch (pkt[3]) {
    case BYD_PACKET_ABSOLUTE:
        if (!priv->touch) {
    ...
}

Should the state be set to PSMOUSE_IGNORE or the RX queue otherwise paused
before we free the private data here?

>  		psmouse->private = NULL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720061259.1601281-1-lilinmao@kylinos.cn?part=1