Re: [PATCH] serdev: ttyport: Clear serport->tty after freeing

[Markus Probst <markus.probst@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]

On Wed, 2026-08-19 at 14:57 +0200, Hans de Goede wrote:
> From: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
> 
> Both error paths in ttyport_open(), and ttyport_close(), release the tty
> with tty_release_struct() and leave serport->tty pointing at freed memory.
> 
> The serdev core itself never dereferences it afterwards. However, a buggy
> driver could easily trigger a use-after-free by calling a ttyport operation
> on a port that is not open, or by calling close() twice. While those
> drivers should be fixed, clearing the pointer, makes them fail
> deterministically instead of touching freed memory.
> 
> Note that a driver which currently double closes gets away with it only by
> chance, depending on whether the freed tty has been reused. After this
> change such a driver oopses immediately instead, which is the intended
> outcome, but it may surface latent bugs elsewhere.
> 
> Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

I recently took the role as maintainer for serdev [1].

Please add me to CC if sending another revision.
I only look loosly at the linux-serial mailing list, so I might
otherwise miss a patch.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/commit/MAINTAINERS?h=driver-core-next&id=6b71fef6f05cee64ebbaa8aec6dc688169f47e9c

> ---
>  drivers/tty/serdev/serdev-ttyport.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index bab1b143b8a6..b6638f9f40d2 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -137,6 +137,7 @@ static int ttyport_open(struct serdev_controller *ctrl)
>  err_unlock:
>  	tty_unlock(tty);
>  	tty_release_struct(tty, serport->tty_idx);
> +	serport->tty = NULL;
>  
>  	return ret;
>  }
> @@ -154,6 +155,7 @@ static void ttyport_close(struct serdev_controller *ctrl)
>  	tty_unlock(tty);
>  
>  	tty_release_struct(tty, serport->tty_idx);
> +	serport->tty = NULL;
>  }
>  
>  static unsigned int ttyport_set_baudrate(struct serdev_controller *ctrl, unsigned int speed)

Always good to see hardening.

Reviewed-by: Markus Probst <markus.probst@posteo.de>

Thanks
- Markus Probst


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]