Re: [PATCH 2/2] serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF
From: Markus Probst
Date: Fri Jul 31 2026 - 09:00:44 EST
On Fri, 2026-07-31 at 10:06 +0200, Greg Kroah-Hartman wrote:
> From: Joshua Rogers <linux@xxxxxxxxx>
>
> ttyport_write_buf() snapshots serport->tty as a raw pointer without
> taking a reference, while ttyport_close() can concurrently release the
> tty via tty_release_struct(), leading to a use-after-free. Use
> tty_port_tty_get() to obtain a reference-counted tty pointer, matching
> the pattern already used by ttyport_write_wakeup().
>
> Assisted-by: AISLE:Snapshot
> Cc: stable <stable@xxxxxxxxxx>
> Signed-off-by: Joshua Rogers <linux@xxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> drivers/tty/serdev/serdev-ttyport.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index 48ce5b3f8308..4d37c8130dd7 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -85,13 +85,21 @@ static const struct tty_port_client_operations client_ops = {
> static ssize_t ttyport_write_buf(struct serdev_controller *ctrl, const u8 *data, size_t len)
> {
> struct serport *serport = serdev_controller_get_drvdata(ctrl);
> - struct tty_struct *tty = serport->tty;
> + struct tty_struct *tty;
> + ssize_t ret;
>
> if (!test_bit(SERPORT_ACTIVE, &serport->flags))
> return 0;
>
> + tty = tty_port_tty_get(serport->port);
> + if (!tty)
> + return 0;
> +
> set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
> - return tty->ops->write(serport->tty, data, len);
> + ret = tty->ops->write(tty, data, len);
> + tty_kref_put(tty);
> +
> + return ret;
> }
>
> static void ttyport_write_flush(struct serdev_controller *ctrl)
It should be the responsibility of the driver to ensure
`serdev_device_close` and `serdev_device_write_buf` are not called
concurrently.
This also applies to
- serdev_device_set_baudrate
- serdev_device_set_flow_control
- serdev_device_wait_until_sent
and more, which currently iirc even introduce a null pointer
dereference if called with the serdev device closed.
I don't see why `serdev_device_write_buf` needs special protection
here, which the other functions don't need for some reason.
As with the previous patch, this still makes sense for hardening.
With scope_guard being used,
Reviewed-by: Markus Probst <markus.probst@xxxxxxxxx>
Thanks
- Markus Probst
Attachment:
signature.asc
Description: This is a digitally signed message part