[PATCH] serial: 8250: validate iomem_base in serial8250_verify_port()

[PATCH] serial: 8250: validate iomem_base in serial8250_verify_port()

[Mingyu Wang]

syzkaller reports a slab-use-after-free in mutex_spin_on_owner().
This occurs because a previous thread crashed during tty_open() while
holding the tty_lock, leaving the owner pointer dangling.

The initial crash is an unhandled Page Fault in mem_serial_in():

  BUG: unable to handle page fault for address: ffffc90001206014
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  Oops: Oops: 0000 [#1] SMP KASAN PTI
  RIP: 0010:mem_serial_in+0x67/0x90
  Call Trace:
   serial8250_clear_interrupts+0x40/0xf0
   serial8250_do_startup+0x3f5/0x3220
   serial8250_startup+0x6c/0x90
   uart_startup+0x4ff/0x12c0
   uart_port_activate+0xf3/0x1a0
   tty_port_open+0x15c/0x200
   uart_open+0x47/0x60
   tty_open+0x39b/0x1310

This sequence is triggered because the TIOCSSERIAL ioctl allows a user
with CAP_SYS_ADMIN to change the io_type of a serial port to UPIO_MEM
(or other memory-mapped types) and inject an arbitrary physical address
via iomem_base.

When uart_change_port() applies these changes, it attempts to remap the
port's I/O memory based on the injected iomem_base. This can result in an
invalid or inaccessible virtual mapping for port->membase. The subsequent
call to mem_serial_in() then dereferences this bogus pointer, triggering
a fatal Page Fault.

Fix this by tightening the validation in serial8250_verify_port().
Reject any TIOCSSERIAL requests that attempt to modify the io_type
to/from memory-mapped types, or change the iomem_base of an existing
memory-mapped port. Userspace can still legitimately modify IRQs or
baud rates of MMIO ports as long as the io_type and mapbase remain
unchanged.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 drivers/tty/serial/8250/8250_port.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index e94a0802cbdd..79563afe9db9 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3146,6 +3146,16 @@ serial8250_verify_port(struct uart_port *port, struct serial_struct *ser)
 	    ser->type >= ARRAY_SIZE(uart_config) || ser->type == PORT_CIRRUS ||
 	    ser->type == PORT_STARTECH)
 		return -EINVAL;
+
+	if (port->iotype == UPIO_MEM || port->iotype == UPIO_MEM32 ||
+	    port->iotype == UPIO_MEM32BE || port->iotype == UPIO_MEM16 ||
+	    ser->io_type == UPIO_MEM || ser->io_type == UPIO_MEM32 ||
+	    ser->io_type == UPIO_MEM32BE || ser->io_type == UPIO_MEM16) {
+		if (ser->io_type != port->iotype ||
+		    (unsigned long)ser->iomem_base != port->mapbase)
+			return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH] serial: 8250: validate iomem_base in serial8250_verify_port()

[Jiri Slaby]

On 20. 07. 26, 7:37, Mingyu Wang wrote:
> Fix this by tightening the validation in serial8250_verify_port().
> Reject any TIOCSSERIAL requests that attempt to modify the io_type
> to/from memory-mapped types, or change the iomem_base of an existing
> memory-mapped port. Userspace can still legitimately modify IRQs or
> baud rates of MMIO ports as long as the io_type and mapbase remain
> unchanged.

Pardon my ignorance, but isn't TIOCSSERIAL intended exactly to change 
the type or mapbase when needed for ports without UPF_FIXED_PORT (and 
under root)?

thanks,
-- 
js
suse labs

Re: [PATCH] serial: 8250: validate iomem_base in serial8250_verify_port()

[Mingyu Wang]

>
> Pardon my ignorance, but isn't TIOCSSERIAL intended exactly to change 
> the type or mapbase when needed for ports without UPF_FIXED_PORT (and 
> under root)?
>
Hi Jiri,

You're right — TIOCSSERIAL is meant to allow root to change io_type
and iomem_base for non-fixed ports. My patch was indeed too restrictive
and would have broken that ABI. That was an oversight on my part.

The issue was identified via local syzkaller testing. It reveals two
related behaviors:
1. Root injects a bogus iomem_base via TIOCSSERIAL; opening the port
    faults in mem_serial_in() because the mapped address is invalid.
2. Because the fault happens while holding tty_lock, the mutex remains
    locked by the dying task. This leads to a cascading use-after-free
    when subsequent threads try to acquire the same lock.

While (1) is effectively "root shooting itself in the foot," (2) results
in cascading failures beyond the initial fault.

How would you prefer to handle this? Should we treat this as wontfix
(since it requires CAP_SYS_ADMIN), or would you prefer a more targeted
check to avoid the cascading UAF (e.g., ensuring basic validity of the
mapbase before ioremap)?

I'm happy to drop this patch if you feel the risk does not justify
additional checks.

thanks,
Mingyu Wang

Re: [PATCH] serial: 8250: validate iomem_base in serial8250_verify_port()

[Greg KH]

On Mon, Jul 20, 2026 at 02:44:19PM +0800, Mingyu Wang wrote:
> 
> > 
> > Pardon my ignorance, but isn't TIOCSSERIAL intended exactly to change
> > the type or mapbase when needed for ports without UPF_FIXED_PORT (and
> > under root)?
> > 
> Hi Jiri,
> 
> You're right — TIOCSSERIAL is meant to allow root to change io_type
> and iomem_base for non-fixed ports. My patch was indeed too restrictive
> and would have broken that ABI. That was an oversight on my part.
> 
> The issue was identified via local syzkaller testing. It reveals two
> related behaviors:
> 1. Root injects a bogus iomem_base via TIOCSSERIAL; opening the port
>    faults in mem_serial_in() because the mapped address is invalid.
> 2. Because the fault happens while holding tty_lock, the mutex remains
>    locked by the dying task. This leads to a cascading use-after-free
>    when subsequent threads try to acquire the same lock.
> 
> While (1) is effectively "root shooting itself in the foot," (2) results
> in cascading failures beyond the initial fault.
> 
> How would you prefer to handle this? Should we treat this as wontfix
> (since it requires CAP_SYS_ADMIN), or would you prefer a more targeted
> check to avoid the cascading UAF (e.g., ensuring basic validity of the
> mapbase before ioremap)?

I think we have been through this before, look in the archives.  syzbot
shouldn't be doing things as root that are known to cause problems with
the system.

It's like it calling 'rm -rf /' from a syzbot script, just don't do
that :)

> I'm happy to drop this patch if you feel the risk does not justify
> additional checks.

We can't break existing apis, sorry.

thanks,

greg k-h

Re: [PATCH] serial: 8250: validate iomem_base in serial8250_verify_port()

[Mingyu Wang]

Understood. Thank you for the clarification. I'll drop this patch.

Best regards,


Mingyu Wang