Re: [PATCH 1/4] Input: psmouse - fix use-after-free during protocol disconnect

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces an unbounded increment of `psmouse->pktcnt` leading to a heap buffer overflow in the `psmouse->packet` array when stray bytes are received during protocol disconnect.
--

--- Patch [1]: [PATCH 1/4] Input: psmouse - fix use-after-free during protocol disconnect ---
commit 6906d0fc401466f9489746b3c6e47ede12a56c49
Author: Dmitry Torokhov <[email protected]>

Input: psmouse - fix use-after-free during protocol disconnect

This commit attempts to fix a use-after-free by explicitly setting
psmouse->protocol_handler to NULL inside a serio_pause_rx scoped_guard.
It also updates psmouse_handle_byte() to drop incoming bytes when the
handler is NULL.

> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index 6ab5f1d96eaea..916a14b9b6a7d 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -267,7 +267,13 @@ void psmouse_set_state(struct psmouse *psmouse, enum psmouse_state new_state)
>   */
>  static int psmouse_handle_byte(struct psmouse *psmouse)
>  {
> -	psmouse_ret_t rc = psmouse->protocol_handler(psmouse);
> +	psmouse_ret_t rc;
> +
> +	/* protocol_handler is NULL when device is being disconnected */
> +	if (unlikely(!psmouse->protocol_handler))
> +		return 0;
> +

[Severity: High]
Does this early return cause an out-of-bounds write in psmouse_receive_byte()?

By returning 0 here, we completely bypass the switch block below which is
responsible for resetting psmouse->pktcnt.

When stray bytes arrive during the disconnect window, psmouse_receive_byte()
will append them to the packet buffer and increment the counter:

psmouse_receive_byte() {
    ...
    psmouse->packet[psmouse->pktcnt++] = data;
    ...
    psmouse_handle_byte(psmouse);
}

If psmouse_handle_byte() keeps returning 0 without resetting pktcnt, won't
pktcnt grow monotonically and eventually overflow the 8-byte packet array?

> +	rc = psmouse->protocol_handler(psmouse);
>  
>  	switch (rc) {
>  	case PSMOUSE_BAD_DATA:

[ ... ]

> @@ -1466,6 +1472,9 @@ static void psmouse_disconnect(struct serio *serio)
>  		psmouse_deactivate(parent);
>  	}
>  
> +	scoped_guard(serio_pause_rx, serio)
> +		psmouse->protocol_handler = NULL;
> +
>  	if (psmouse->disconnect)
>  		psmouse->disconnect(psmouse);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1