[PATCH 0/2] serdev: Some small serdev bugfixes found by code scans

[PATCH 0/2] serdev: Some small serdev bugfixes found by code scans

[Greg Kroah-Hartman]

Joshua at AISLE has been kind enough to run a bunch of scans on the
tty/serial code and here are two minor fixes for the vt code that the
tools found.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Joshua Rogers (2):
      serdev: fix race between tty-port unregister and in-flight callbacks
      serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF

 drivers/tty/serdev/serdev-ttyport.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)
---
base-commit: 8ba098e6b6ff0db8edf28528d1552be261af30d4
change-id: 20260731-aisle-tty-serdev-8b7daaf0a7f6

Best regards,
--  
Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH 1/2] serdev: fix race between tty-port unregister and in-flight callbacks

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

serdev_tty_port_unregister() clears port->client_data and frees the
controller without synchronizing with in-flight flip buffer work.
This can cause NULL pointer dereferences or use-after-free if
ttyport_receive_buf() or ttyport_write_wakeup() runs concurrently.

Add cancel_work_sync() to drain pending buffer work before clearing
state, and add NULL checks for client_data in both callbacks as
secondary hardening.

Assisted-by: AISLE:Snapshot
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index bab1b143b8a6..48ce5b3f8308 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -26,9 +26,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
 				  const u8 *fp, size_t count)
 {
 	struct serdev_controller *ctrl = port->client_data;
-	struct serport *serport = serdev_controller_get_drvdata(ctrl);
+	struct serport *serport;
 	size_t ret;
 
+	if (!ctrl)
+		return 0;
+
+	serport = serdev_controller_get_drvdata(ctrl);
+
 	if (!test_bit(SERPORT_ACTIVE, &serport->flags))
 		return 0;
 
@@ -46,9 +51,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
 static void ttyport_write_wakeup(struct tty_port *port)
 {
 	struct serdev_controller *ctrl = port->client_data;
-	struct serport *serport = serdev_controller_get_drvdata(ctrl);
+	struct serport *serport;
 	struct tty_struct *tty;
 
+	if (!ctrl)
+		return;
+
+	serport = serdev_controller_get_drvdata(ctrl);
+
 	tty = tty_port_tty_get(port);
 	if (!tty)
 		return;
@@ -312,6 +322,7 @@ int serdev_tty_port_unregister(struct tty_port *port)
 		return -ENODEV;
 
 	serdev_controller_remove(ctrl);
+	cancel_work_sync(&port->buf.work);
 	port->client_data = NULL;
 	port->client_ops = &tty_port_default_client_ops;
 	serdev_controller_put(ctrl);

-- 
2.55.0

[PATCH 2/2] serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

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@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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)

-- 
2.55.0

Re: [PATCH 0/2] serdev: Some small serdev bugfixes found by code scans

[Greg Kroah-Hartman]

On Fri, Jul 31, 2026 at 10:06:08AM +0200, Greg Kroah-Hartman wrote:
> Joshua at AISLE has been kind enough to run a bunch of scans on the
> tty/serial code and here are two minor fixes for the vt code that the

s/vt/serdev/

{sigh}

Re: [PATCH 2/2] serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF

[Jiri Slaby]

On 31. 07. 26, 10:06, Greg Kroah-Hartman wrote:
> From: Joshua Rogers <linux@joshua.hu>
> 
> 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@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   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);

Here,

   scope_guard(tty_port_tty, serport->port) {
     struct tty_struct *tty = scoped_tty();

     set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
     return tty->ops->write(serport->tty, data, len);
   }

   return 0;

appears to be cleaner.

thanks,
-- 
js
suse labs

Re: [PATCH 2/2] serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF

[Greg Kroah-Hartman]

On Fri, Jul 31, 2026 at 10:24:02AM +0200, Jiri Slaby wrote:
> On 31. 07. 26, 10:06, Greg Kroah-Hartman wrote:
> > From: Joshua Rogers <linux@joshua.hu>
> > 
> > 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@kernel.org>
> > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >   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);
> 
> Here,
> 
>   scope_guard(tty_port_tty, serport->port) {
>     struct tty_struct *tty = scoped_tty();
> 
>     set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
>     return tty->ops->write(serport->tty, data, len);
>   }
> 
>   return 0;
> 
> appears to be cleaner.

Yes, much cleaner.  LLMs really don't know about "modern" kernel coding
styles (they always use min_t() and don't like guard() code).

I'll respin this and do a new version in a few days, thanks.

greg k-h

Re: [PATCH 1/2] serdev: fix race between tty-port unregister and in-flight callbacks

[Markus Probst]

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

On Fri, 2026-07-31 at 10:06 +0200, Greg Kroah-Hartman wrote:
> From: Joshua Rogers <linux@joshua.hu>
> 
> serdev_tty_port_unregister() clears port->client_data and frees the
> controller without synchronizing with in-flight flip buffer work.
> This can cause NULL pointer dereferences or use-after-free if
> ttyport_receive_buf() or ttyport_write_wakeup() runs concurrently.
> 
> Add cancel_work_sync() to drain pending buffer work before clearing
> state, and add NULL checks for client_data in both callbacks as
> secondary hardening.
> 
> Assisted-by: AISLE:Snapshot
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index bab1b143b8a6..48ce5b3f8308 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -26,9 +26,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
>  				  const u8 *fp, size_t count)
>  {
>  	struct serdev_controller *ctrl = port->client_data;
> -	struct serport *serport = serdev_controller_get_drvdata(ctrl);
> +	struct serport *serport;
>  	size_t ret;
>  
> +	if (!ctrl)
> +		return 0;
> +
> +	serport = serdev_controller_get_drvdata(ctrl);
> +
>  	if (!test_bit(SERPORT_ACTIVE, &serport->flags))
>  		return 0;
>  
> @@ -46,9 +51,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
>  static void ttyport_write_wakeup(struct tty_port *port)
>  {
>  	struct serdev_controller *ctrl = port->client_data;
> -	struct serport *serport = serdev_controller_get_drvdata(ctrl);
> +	struct serport *serport;
>  	struct tty_struct *tty;
>  
> +	if (!ctrl)
> +		return;
> +
> +	serport = serdev_controller_get_drvdata(ctrl);
> +
>  	tty = tty_port_tty_get(port);
>  	if (!tty)
>  		return;
> @@ -312,6 +322,7 @@ int serdev_tty_port_unregister(struct tty_port *port)
>  		return -ENODEV;
>  
>  	serdev_controller_remove(ctrl);
> +	cancel_work_sync(&port->buf.work);
>  	port->client_data = NULL;
>  	port->client_ops = &tty_port_default_client_ops;
>  	serdev_controller_put(ctrl);

So why exactly does tty keep calling `receive_buf` and `write_wakeup`
with the tty port closed?

After `serdev_controller_remove` is called, the tty port should already
be closed by the drivers.

Thanks
- Markus Probst


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

Re: [PATCH 1/2] serdev: fix race between tty-port unregister and in-flight callbacks

[Greg Kroah-Hartman]

On Fri, Jul 31, 2026 at 11:30:53AM +0000, Markus Probst wrote:
> On Fri, 2026-07-31 at 10:06 +0200, Greg Kroah-Hartman wrote:
> > From: Joshua Rogers <linux@joshua.hu>
> > 
> > serdev_tty_port_unregister() clears port->client_data and frees the
> > controller without synchronizing with in-flight flip buffer work.
> > This can cause NULL pointer dereferences or use-after-free if
> > ttyport_receive_buf() or ttyport_write_wakeup() runs concurrently.
> > 
> > Add cancel_work_sync() to drain pending buffer work before clearing
> > state, and add NULL checks for client_data in both callbacks as
> > secondary hardening.
> > 
> > Assisted-by: AISLE:Snapshot
> > Cc: stable <stable@kernel.org>
> > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> > index bab1b143b8a6..48ce5b3f8308 100644
> > --- a/drivers/tty/serdev/serdev-ttyport.c
> > +++ b/drivers/tty/serdev/serdev-ttyport.c
> > @@ -26,9 +26,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
> >  				  const u8 *fp, size_t count)
> >  {
> >  	struct serdev_controller *ctrl = port->client_data;
> > -	struct serport *serport = serdev_controller_get_drvdata(ctrl);
> > +	struct serport *serport;
> >  	size_t ret;
> >  
> > +	if (!ctrl)
> > +		return 0;
> > +
> > +	serport = serdev_controller_get_drvdata(ctrl);
> > +
> >  	if (!test_bit(SERPORT_ACTIVE, &serport->flags))
> >  		return 0;
> >  
> > @@ -46,9 +51,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
> >  static void ttyport_write_wakeup(struct tty_port *port)
> >  {
> >  	struct serdev_controller *ctrl = port->client_data;
> > -	struct serport *serport = serdev_controller_get_drvdata(ctrl);
> > +	struct serport *serport;
> >  	struct tty_struct *tty;
> >  
> > +	if (!ctrl)
> > +		return;
> > +
> > +	serport = serdev_controller_get_drvdata(ctrl);
> > +
> >  	tty = tty_port_tty_get(port);
> >  	if (!tty)
> >  		return;
> > @@ -312,6 +322,7 @@ int serdev_tty_port_unregister(struct tty_port *port)
> >  		return -ENODEV;
> >  
> >  	serdev_controller_remove(ctrl);
> > +	cancel_work_sync(&port->buf.work);
> >  	port->client_data = NULL;
> >  	port->client_ops = &tty_port_default_client_ops;
> >  	serdev_controller_put(ctrl);
> 
> So why exactly does tty keep calling `receive_buf` and `write_wakeup`
> with the tty port closed?
> 
> After `serdev_controller_remove` is called, the tty port should already
> be closed by the drivers.

Are you sure?  remove can be called by a device removal, while close
might be coming from a different code path (like it is in userspace.)

If this is impossible, as close will ALWAYS happen before remove, then
it's not an issue, but that is probably not the case, right?

thanks,

greg k-h

Re: [PATCH 1/2] serdev: fix race between tty-port unregister and in-flight callbacks

[Markus Probst]

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

On Fri, 2026-07-31 at 14:01 +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 31, 2026 at 11:30:53AM +0000, Markus Probst wrote:
> > On Fri, 2026-07-31 at 10:06 +0200, Greg Kroah-Hartman wrote:
> > > From: Joshua Rogers <linux@joshua.hu>
> > > 
> > > serdev_tty_port_unregister() clears port->client_data and frees the
> > > controller without synchronizing with in-flight flip buffer work.
> > > This can cause NULL pointer dereferences or use-after-free if
> > > ttyport_receive_buf() or ttyport_write_wakeup() runs concurrently.
> > > 
> > > Add cancel_work_sync() to drain pending buffer work before clearing
> > > state, and add NULL checks for client_data in both callbacks as
> > > secondary hardening.
> > > 
> > > Assisted-by: AISLE:Snapshot
> > > Cc: stable <stable@kernel.org>
> > > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > ---
> > >  drivers/tty/serdev/serdev-ttyport.c | 15 +++++++++++++--
> > >  1 file changed, 13 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> > > index bab1b143b8a6..48ce5b3f8308 100644
> > > --- a/drivers/tty/serdev/serdev-ttyport.c
> > > +++ b/drivers/tty/serdev/serdev-ttyport.c
> > > @@ -26,9 +26,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
> > >  				  const u8 *fp, size_t count)
> > >  {
> > >  	struct serdev_controller *ctrl = port->client_data;
> > > -	struct serport *serport = serdev_controller_get_drvdata(ctrl);
> > > +	struct serport *serport;
> > >  	size_t ret;
> > >  
> > > +	if (!ctrl)
> > > +		return 0;
> > > +
> > > +	serport = serdev_controller_get_drvdata(ctrl);
> > > +
> > >  	if (!test_bit(SERPORT_ACTIVE, &serport->flags))
> > >  		return 0;
> > >  
> > > @@ -46,9 +51,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
> > >  static void ttyport_write_wakeup(struct tty_port *port)
> > >  {
> > >  	struct serdev_controller *ctrl = port->client_data;
> > > -	struct serport *serport = serdev_controller_get_drvdata(ctrl);
> > > +	struct serport *serport;
> > >  	struct tty_struct *tty;
> > >  
> > > +	if (!ctrl)
> > > +		return;
> > > +
> > > +	serport = serdev_controller_get_drvdata(ctrl);
> > > +
> > >  	tty = tty_port_tty_get(port);
> > >  	if (!tty)
> > >  		return;
> > > @@ -312,6 +322,7 @@ int serdev_tty_port_unregister(struct tty_port *port)
> > >  		return -ENODEV;
> > >  
> > >  	serdev_controller_remove(ctrl);
> > > +	cancel_work_sync(&port->buf.work);
> > >  	port->client_data = NULL;
> > >  	port->client_ops = &tty_port_default_client_ops;
> > >  	serdev_controller_put(ctrl);
> > 
> > So why exactly does tty keep calling `receive_buf` and `write_wakeup`
> > with the tty port closed?
> > 
> > After `serdev_controller_remove` is called, the tty port should already
> > be closed by the drivers.
> 
> Are you sure?  remove can be called by a device removal, while close
> might be coming from a different code path (like it is in userspace.)
> 
> If this is impossible, as close will ALWAYS happen before remove, then
> it's not an issue, but that is probably not the case, right?

The driver *should* always close the serdev device on remove if
previously opened. Thus the existence of `devm_serdev_device_open`.

The serdev subsystem itself does not guarantee that it has been called.

I just took a look at every driver using `serdev_device_open` (non
devm), and every one seems to call `serdev_device_close` on remove.

Assuming `cancel_work_sync(&port->buf.work);` is used correctly here,
we could still take the patch for hardening.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

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

Re: [PATCH 2/2] serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF

[Markus Probst]

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

On Fri, 2026-07-31 at 10:06 +0200, Greg Kroah-Hartman wrote:
> From: Joshua Rogers <linux@joshua.hu>
> 
> 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@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  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@posteo.de>


Thanks
- Markus Probst


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

Re: [PATCH 2/2] serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF

[Greg Kroah-Hartman]

On Fri, Jul 31, 2026 at 01:00:30PM +0000, Markus Probst wrote:
> On Fri, 2026-07-31 at 10:06 +0200, Greg Kroah-Hartman wrote:
> > From: Joshua Rogers <linux@joshua.hu>
> > 
> > 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@kernel.org>
> > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  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

Doesn't touch a tty structure, so why is the same reference count logic
needed here?

> - serdev_device_set_flow_control

Same here.

> - serdev_device_wait_until_sent

Or here?

> 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.

It's the access to the tty pointer that I think the issue is, right?

> As with the previous patch, this still makes sense for hardening.
> With scope_guard being used,
> 
> Reviewed-by: Markus Probst <markus.probst@posteo.de>

Great, thanks, I'll send out a v2 next week with that change made, as
I've done it locally and will wait a bit.

thanks,

greg k-h

Re: [PATCH 2/2] serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF

[Markus Probst]

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

On Fri, 2026-07-31 at 15:22 +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 31, 2026 at 01:00:30PM +0000, Markus Probst wrote:
> > On Fri, 2026-07-31 at 10:06 +0200, Greg Kroah-Hartman wrote:
> > > From: Joshua Rogers <linux@joshua.hu>
> > > 
> > > 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@kernel.org>
> > > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > ---
> > >  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
> 
> Doesn't touch a tty structure, so why is the same reference count logic
> needed here?
It calls `ttyport_set_baudrate`.

> 
> > - serdev_device_set_flow_control
It calls `ttyport_set_flow_control`.
> 
> Same here.
> 
> > - serdev_device_wait_until_sent
> 
> Or here?
It calls `ttyport_wait_until_sent`.

See `struct serdev_controller_ops ctrl_ops` in `serdev-ttyport.c`.

All of those mentioned above call into tty_ functions. Obviously the
tty_struct pointer needs to be valid for those calls, which is only the
case if the device is open.

Thanks
- Markus Probst

> 
> > 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.
> 
> It's the access to the tty pointer that I think the issue is, right?
> 
> > As with the previous patch, this still makes sense for hardening.
> > With scope_guard being used,
> > 
> > Reviewed-by: Markus Probst <markus.probst@posteo.de>
> 
> Great, thanks, I'll send out a v2 next week with that change made, as
> I've done it locally and will wait a bit.
> 
> thanks,
> 
> greg k-h

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