[PATCH] serial: 8250: clear RX trigger callbacks when removing a port

[PATCH] serial: 8250: clear RX trigger callbacks when removing a port

[James Hilliard]

serial8250_unregister_port() removes the device supplied by a driver and
normally re-registers the persistent slot as an unknown legacy port. The
slot retains its RX trigger callbacks and sysfs attribute group, so the
replacement rx_trig_bytes attribute can still call the removed driver's
code. Unloading that driver leaves the callbacks pointing into freed
module text.

Clear both callbacks after uart_remove_one_port() has drained the old
sysfs users and before registering the fallback port. Its RX trigger
attribute then uses the generic handlers, which reject a port without
FIFO capabilities.

Also copy the callbacks unconditionally during registration. A replacement
port without custom handlers must use the generic implementation instead
of inheriting callbacks from the previous owner of the slot.

Fixes: 8a6c543cf8b3 ("serial: 8250: allow UART drivers to override rx_trig_bytes handling")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/tty/serial/8250/8250_core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 42319843cb5b..c638e10c43e2 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -830,10 +830,8 @@ static int __serial8250_register_8250_port(struct uart_8250_port *uart,
 		uart->port.get_divisor = up->port.get_divisor;
 	if (up->port.set_divisor)
 		uart->port.set_divisor = up->port.set_divisor;
-	if (up->port.get_rxtrig)
-		uart->port.get_rxtrig = up->port.get_rxtrig;
-	if (up->port.set_rxtrig)
-		uart->port.set_rxtrig = up->port.set_rxtrig;
+	uart->port.get_rxtrig = up->port.get_rxtrig;
+	uart->port.set_rxtrig = up->port.set_rxtrig;
 	if (up->port.startup)
 		uart->port.startup = up->port.startup;
 	if (up->port.shutdown)
@@ -953,6 +951,9 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
 	}
 
 	uart_remove_one_port(&serial8250_reg, &uart->port);
+	/* The fallback port must use the generic RX trigger handlers. */
+	uart->port.get_rxtrig = NULL;
+	uart->port.set_rxtrig = NULL;
 	if (serial8250_isa_devs) {
 		uart->port.flags &= ~UPF_BOOT_AUTOCONF;
 		uart->port.type = PORT_UNKNOWN;

---
base-commit: 1184f87e2790e8af9be3116f608d2e86b4792d7d
change-id: 20261024-submit-serial-8250-rxtrig-lifetime-v1-4fa940207ae5

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

Re: [PATCH] serial: 8250: clear RX trigger callbacks when removing a port

[Jiri Slaby]

On 24. 10. 26, 22:05, James Hilliard wrote:
> serial8250_unregister_port() removes the device supplied by a driver and
> normally re-registers the persistent slot as an unknown legacy port. The
> slot retains its RX trigger callbacks and sysfs attribute group, so the
> replacement rx_trig_bytes attribute can still call the removed driver's
> code. Unloading that driver leaves the callbacks pointing into freed
> module text.
> 
> Clear both callbacks after uart_remove_one_port() has drained the old
> sysfs users and before registering the fallback port. Its RX trigger
> attribute then uses the generic handlers, which reject a port without
> FIFO capabilities.
> 
> Also copy the callbacks unconditionally during registration. A replacement
> port without custom handlers must use the generic implementation instead
> of inheriting callbacks from the previous owner of the slot.

I don't understand why this is a problem only for the two callbacks. 
Care to elaborate?

> Fixes: 8a6c543cf8b3 ("serial: 8250: allow UART drivers to override rx_trig_bytes handling")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   drivers/tty/serial/8250/8250_core.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 42319843cb5b..c638e10c43e2 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -830,10 +830,8 @@ static int __serial8250_register_8250_port(struct uart_8250_port *uart,
>   		uart->port.get_divisor = up->port.get_divisor;
>   	if (up->port.set_divisor)
>   		uart->port.set_divisor = up->port.set_divisor;
> -	if (up->port.get_rxtrig)
> -		uart->port.get_rxtrig = up->port.get_rxtrig;
> -	if (up->port.set_rxtrig)
> -		uart->port.set_rxtrig = up->port.set_rxtrig;
> +	uart->port.get_rxtrig = up->port.get_rxtrig;
> +	uart->port.set_rxtrig = up->port.set_rxtrig;
>   	if (up->port.startup)
>   		uart->port.startup = up->port.startup;
>   	if (up->port.shutdown)
> @@ -953,6 +951,9 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
>   	}
>   
>   	uart_remove_one_port(&serial8250_reg, &uart->port);
> +	/* The fallback port must use the generic RX trigger handlers. */
> +	uart->port.get_rxtrig = NULL;
> +	uart->port.set_rxtrig = NULL;
>   	if (serial8250_isa_devs) {
>   		uart->port.flags &= ~UPF_BOOT_AUTOCONF;
>   		uart->port.type = PORT_UNKNOWN;

thanks,
-- 
js
suse labs

Re: [PATCH] serial: 8250: clear RX trigger callbacks when removing a port

[James Hilliard]

On Tue, Oct 27, 2026 at 12:09 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 24. 10. 26, 22:05, James Hilliard wrote:
> > serial8250_unregister_port() removes the device supplied by a driver and
> > normally re-registers the persistent slot as an unknown legacy port. The
> > slot retains its RX trigger callbacks and sysfs attribute group, so the
> > replacement rx_trig_bytes attribute can still call the removed driver's
> > code. Unloading that driver leaves the callbacks pointing into freed
> > module text.
> >
> > Clear both callbacks after uart_remove_one_port() has drained the old
> > sysfs users and before registering the fallback port. Its RX trigger
> > attribute then uses the generic handlers, which reject a port without
> > FIFO capabilities.
> >
> > Also copy the callbacks unconditionally during registration. A replacement
> > port without custom handlers must use the generic implementation instead
> > of inheriting callbacks from the previous owner of the slot.
>
> I don't understand why this is a problem only for the two callbacks.
> Care to elaborate?

It isn't limited to those two callbacks. The specific path here is the
rx_trig_bytes attribute recreated for the fallback PORT_UNKNOWN port.
Its show/store handlers call get_rxtrig/set_rxtrig directly, without
requiring an opened or initialized port. The FIFO capability checks are
inside the generic handlers, so retaining a custom callback bypasses
those checks.

The other callbacks have different protections: serial8250_init_port()
clears pm, for example, and uart_port_startup() skips startup for
PORT_UNKNOWN. Those protections do not cover subsequent slot reuse.

In that replacement case, startup and set_termios are also copied only
when the new driver supplies them, and serial8250_set_defaults() does
not reset them. They can therefore retain the previous owner's handlers
too.

This patch addresses the direct sysfs exposure introduced by the
referenced commit, including replacement of these callbacks when the
slot is reused. The other retained callbacks need a broader cleanup of
the replacement path as well.

>
> > Fixes: 8a6c543cf8b3 ("serial: 8250: allow UART drivers to override rx_trig_bytes handling")
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> >   drivers/tty/serial/8250/8250_core.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> > index 42319843cb5b..c638e10c43e2 100644
> > --- a/drivers/tty/serial/8250/8250_core.c
> > +++ b/drivers/tty/serial/8250/8250_core.c
> > @@ -830,10 +830,8 @@ static int __serial8250_register_8250_port(struct uart_8250_port *uart,
> >               uart->port.get_divisor = up->port.get_divisor;
> >       if (up->port.set_divisor)
> >               uart->port.set_divisor = up->port.set_divisor;
> > -     if (up->port.get_rxtrig)
> > -             uart->port.get_rxtrig = up->port.get_rxtrig;
> > -     if (up->port.set_rxtrig)
> > -             uart->port.set_rxtrig = up->port.set_rxtrig;
> > +     uart->port.get_rxtrig = up->port.get_rxtrig;
> > +     uart->port.set_rxtrig = up->port.set_rxtrig;
> >       if (up->port.startup)
> >               uart->port.startup = up->port.startup;
> >       if (up->port.shutdown)
> > @@ -953,6 +951,9 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
> >       }
> >
> >       uart_remove_one_port(&serial8250_reg, &uart->port);
> > +     /* The fallback port must use the generic RX trigger handlers. */
> > +     uart->port.get_rxtrig = NULL;
> > +     uart->port.set_rxtrig = NULL;
> >       if (serial8250_isa_devs) {
> >               uart->port.flags &= ~UPF_BOOT_AUTOCONF;
> >               uart->port.type = PORT_UNKNOWN;
>
> thanks,
> --
> js
> suse labs