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

[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: sta...@vger.kernel.org
Signed-off-by: Mingyu Wang <25181...@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

[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

[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

[Mingyu Wang]

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

Best regards,


Mingyu Wang