[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