Re: [PATCH 05/24] serial: 8250: clear subdriver state on port unregister

[James Hilliard]

On Tue, Oct 27, 2026 at 5:16 AM Jiri Slaby <jirislaby@xxxxxxxxxx> wrote:
>
> On 27. 10. 26, 12:06, Greg Kroah-Hartman wrote:
> > From: Joshua Rogers <linux@xxxxxxxxx>
> >
> > When serial8250_unregister_port() re-adds a port as a fallback ISA port,
> > it only partially resets the port state via serial8250_init_port(). All
> > subdriver-owned function pointers and per-driver state copied during
> > registration (such as startup, shutdown, handle_break, get_mctrl,
> > set_mctrl, set_termios, private_data, dma, gpios, and rs485 callbacks)
> > are left intact. After the original driver module is unloaded, these
> > become dangling pointers that can trigger use-after-free on subsequent
> > port operations.
> >
> > Clear all subdriver-copied fields and call serial8250_set_defaults() to
> > restore default I/O accessors before re-adding the fallback port.
> >
> > Cc: stable <stable@xxxxxxxxxx>
> > Cc: Jiri Slaby <jirislaby@xxxxxxxxxx>
> > Cc: John Ogness <john.ogness@xxxxxxxxxxxxx>
> > Cc: Crescent Hsieh <crescentcy.hsieh@xxxxxxxx>
> > Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@xxxxxxxxxxxx>
> > Cc: Fushuai Wang <wangfushuai@xxxxxxxxx>
> > Signed-off-by: Joshua Rogers <linux@xxxxxxxxx>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > ---
> >   drivers/tty/serial/8250/8250_core.c | 28 ++++++++++++++++++++++++++++
> >   1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> > index 0951d3e42e1f..673fde55828a 100644
> > --- a/drivers/tty/serial/8250/8250_core.c
> > +++ b/drivers/tty/serial/8250/8250_core.c
> > @@ -956,7 +956,35 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
> >               uart->port.dev = &serial8250_isa_devs->dev;
> >               uart->port.port_id = uart->port.line;
> >               uart->capabilities = 0;
> > +
> > +             /*
> > +              * Clear subdriver-owned state copied by
> > +              * serial8250_register_8250_port() to prevent dangling
> > +              * pointers after the original driver module is unloaded.
> > +              */
> > +             uart->port.private_data = NULL;
> > +             uart->port.set_termios = NULL;
> > +             uart->port.set_ldisc = NULL;
> > +             uart->port.get_mctrl = NULL;
> > +             uart->port.set_mctrl = NULL;
> > +             uart->port.get_divisor = NULL;
> > +             uart->port.set_divisor = NULL;
> > +             uart->port.startup = NULL;
> > +             uart->port.shutdown = NULL;
> > +             uart->port.throttle = NULL;
> > +             uart->port.unthrottle = NULL;
> > +             uart->port.handle_break = NULL;
> > +             uart->port.rs485_config = NULL;
> > +             memset(&uart->port.rs485, 0, sizeof(uart->port.rs485));
> > +             memset(&uart->port.rs485_supported, 0,
> > +                    sizeof(uart->port.rs485_supported));
> > +             uart->gpios = NULL;
> > +             uart->dma = NULL;
> > +             uart->rs485_start_tx = NULL;
> > +             uart->rs485_stop_tx = NULL;
> > +
>
> Ha! The above gives an answer to my today's:
> https://lore.kernel.org/all/53624662-e94e-4fcb-9902-7fa4143c7689@xxxxxxxxxx/

Yes, this addresses the broader cleanup issue. My RX-trigger fix [0]
complements it by clearing get_rxtrig and set_rxtrig after
uart_remove_one_port(), before registering the fallback port.

It also copies those callbacks unconditionally during registration,
including NULL, so a replacement without custom handlers cannot inherit
the previous owner's callbacks.

[0] https://lore.kernel.org/r/20261024-submit-serial-8250-rxtrig-lifetime-v1-v1-1-731b78d5003d@xxxxxxxxx/

>
> (And obviously this conflicts with that patch.)
>
>
> And with the current state of LLMs, I should perhaps revive my "move ops
> to a separate struct" as the above is a PITA. I have been unable to
> finish the series on my own to date.
>
> >               serial8250_init_port(uart);
> > +             serial8250_set_defaults(uart);
> >               serial8250_apply_quirks(uart);
> >               uart_add_one_port(&serial8250_reg, &uart->port);
> >       } else {
> >
>
> thanks,
> --
> js
> suse labs
>