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

[sashiko-bot@kernel.org]

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