Linux Serial subsystem development
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Markus Probst <markus.probst@posteo.de>
Cc: linux-serial@vger.kernel.org, Rob Herring <robh@kernel.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	linux-kernel@vger.kernel.org, Joshua Rogers <linux@joshua.hu>,
	stable <stable@kernel.org>
Subject: Re: [PATCH 1/2] serdev: fix race between tty-port unregister and in-flight callbacks
Date: Fri, 31 Jul 2026 14:01:12 +0200	[thread overview]
Message-ID: <2026073118-handwash-vitality-5f3f@gregkh> (raw)
In-Reply-To: <edd75d5a4362e5c8e7b691f65768d1fedc30309a.camel@posteo.de>

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

  reply	other threads:[~2026-07-31 12:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31  8:06 [PATCH 0/2] serdev: Some small serdev bugfixes found by code scans Greg Kroah-Hartman
2026-07-31  8:06 ` [PATCH 1/2] serdev: fix race between tty-port unregister and in-flight callbacks Greg Kroah-Hartman
2026-07-31 11:30   ` Markus Probst
2026-07-31 12:01     ` Greg Kroah-Hartman [this message]
2026-07-31 12:43       ` Markus Probst
2026-07-31  8:06 ` [PATCH 2/2] serdev: use tty_port_tty_get() in ttyport_write_buf() to prevent UAF Greg Kroah-Hartman
2026-07-31  8:24   ` Jiri Slaby
2026-07-31  8:28     ` Greg Kroah-Hartman
2026-07-31 13:00   ` Markus Probst
2026-07-31 13:22     ` Greg Kroah-Hartman
2026-07-31 13:30       ` Markus Probst
2026-07-31  8:14 ` [PATCH 0/2] serdev: Some small serdev bugfixes found by code scans Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026073118-handwash-vitality-5f3f@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@joshua.hu \
    --cc=markus.probst@posteo.de \
    --cc=robh@kernel.org \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox