Thread (13 messages) 13 messages, 3 authors, 5d ago
COOLING5d

[PATCH v6] serial: 8250: fix use-after-free in IRQ chain handling

From: Qiliang Yuan <hidden>
Date: 2026-07-07 14:06:29
Also in: lkml
Subsystem: 8250/16?50 (and clone uarts) serial driver, the rest, tty layer and serial drivers · Maintainers: Greg Kroah-Hartman, Linus Torvalds, Jiri Slaby

serial_unlink_irq_chain() holds hash_mutex and calls free_irq() + kfree(i)
on an empty port list.  serial_link_irq_chain() used to release hash_mutex
after serial_get_or_create_irq_info() but before acquiring i->lock, so a
concurrent unlink could see list_empty() as true while a port was being
added, free i, and cause a use-after-free.

Dropping hash_mutex before request_irq() completes also lets another port
on the same IRQ join the chain and run the THRE test while IRQ startup is
still in progress, triggering "Unbalanced enable for IRQ" because
irq_shutdown() in the premature free_irq() path increments desc->depth and
breaks the disable_irq/enable_irq pairing in serial8250_THRE_test().

Hold hash_mutex across the whole first request_irq() completion (including
the error cleanup path) by taking it at the top of serial_link_irq_chain()
via guard(mutex)(&hash_mutex).  serial_unlink_irq_chain() already holds
hash_mutex throughout, so the race is closed.

With hash_mutex now taken by the caller, replace the guard in
serial_get_or_create_irq_info() with lockdep_assert_held() and add
__must_hold(&hash_mutex) for static analysis.

Fixes: 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and RT kernels")
Reported-by: Wang Zhaolong <redacted>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Signed-off-by: Qiliang Yuan <redacted>
---
V5 -> V6:
- Keep guard(mutex) and scoped_guard(spinlock_irq) style instead of
  manual locking; the lock lifecycle fits the scope-based model.

V4 -> V5:
- Add __must_hold(&hash_mutex) annotation to
  serial_get_or_create_irq_info() for static analysis.

V3 -> V4:
- Move cleanup under hash_mutex on request_irq() failure to prevent a
  second port from joining the chain before the irq_info is cleaned up.
- Fix inaccurate description of irq_shutdown() in commit message.

V2 -> V3:
- Hold hash_mutex across the first request_irq() completion to prevent a
  second port from joining the chain and running the shared-IRQ THRE test
  while IRQ startup is still in progress.

V1 -> V2:
- Add Reported-by tag from Wang Zhaolong.

v5: https://lore.kernel.org/r/20260624-bug-221579-8250-shared-irq-race-v5-1-15d841f89e1e@gmail.com (local)
v4: https://lore.kernel.org/r/20260529-bug-221579-8250-shared-irq-race-v4-1-cfda63b4420f@gmail.com (local)
v3: https://lore.kernel.org/r/20260529-bug-221579-8250-shared-irq-race-v3-1-fe4d430862a9@gmail.com (local)
v2: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v2-1-06531202e54d@gmail.com (local)
v1: https://lore.kernel.org/r/20260528-bug-221579-8250-shared-irq-race-v1-1-30980cca02f3@gmail.com (local)
---
 drivers/tty/serial/8250/8250_core.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb7..6fd3bb2eee233 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -131,10 +131,11 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
  * - allocate a new one, add it to the hashtable and return it.
  */
 static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
+	__must_hold(&hash_mutex)
 {
 	struct irq_info *i;
 
-	guard(mutex)(&hash_mutex);
+	lockdep_assert_held(&hash_mutex);
 
 	hash_for_each_possible(irq_lists, i, node, up->port.irq)
 		if (i->irq == up->port.irq)
@@ -151,19 +152,37 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
 	return i;
 }
 
+/*
+ * serial_link_irq_chain() hooks the given 8250 port into the IRQ chain.
+ *
+ * hash_mutex must be held from the hash lookup through the first
+ * request_irq() completion.  Dropping it earlier allows a concurrent
+ * serial_unlink_irq_chain() to race in after i->head is published but
+ * before the IRQ is fully set up  another port sharing the IRQ can then
+ * join the chain and run the shared-IRQ THRE test while IRQ startup is
+ * still in progress, triggering an "Unbalanced enable for IRQ" warning
+ * in kernel/irq/manage.c.
+ */
 static int serial_link_irq_chain(struct uart_8250_port *up)
 {
 	struct irq_info *i;
 	int ret;
 
+	guard(mutex)(&hash_mutex);
+
 	i = serial_get_or_create_irq_info(up);
 	if (IS_ERR(i))
 		return PTR_ERR(i);
 
+	/*
+	 * Serialise against the list manipulation in the interrupt handler
+	 * and in serial_unlink_irq_chain().  hash_mutex is still held which
+	 * prevents serial_unlink_irq_chain() from entering and freeing the
+	 * irq_info until the first request_irq() completes.
+	 */
 	scoped_guard(spinlock_irq, &i->lock) {
 		if (i->head) {
 			list_add(&up->list, i->head);
-
 			return 0;
 		}
 
@@ -171,11 +190,14 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
 		i->head = &up->list;
 	}
 
-	ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
-	if (ret < 0)
+	ret = request_irq(up->port.irq, serial8250_interrupt,
+			  up->port.irqflags, up->port.name, i);
+	if (ret < 0) {
 		serial_do_unlink(i, up);
+		return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static void serial_unlink_irq_chain(struct uart_8250_port *up)
---
base-commit: eb3f4b7426cfd2b79d65b7d37155480b32259a11
change-id: 20260528-bug-221579-8250-shared-irq-race-581e4900a178

Best regards,
-- 
Jing Wu [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help