[PATCH] serial: 8250: serialize shared IRQ startup

[PATCH] serial: 8250: serialize shared IRQ startup

[Wang Zhaolong]

Concurrent startup of two 8250 ports sharing the same IRQ can trigger an
IRQ core warning:

  Unbalanced enable for IRQ 3
  WARNING: CPU: 0 PID: 580 at kernel/irq/manage.c:774 __enable_irq+0x3b/0x60
  Call Trace:
   enable_irq+0x8d/0x120
   serial8250_do_startup+0x80d/0xa80
   uart_port_startup+0x13d/0x440
   uart_port_activate+0x5b/0xb0
   tty_port_open+0xa1/0x120
   uart_open+0x1e/0x30
   tty_open+0x140/0x7a0

The second port can then run the shared-IRQ startup test while the IRQ core
is still enabling the line for the first port.  The local
disable_irq_nosync()/enable_irq() pair is balanced, but the interleaving can
still unbalance the IRQ core disable depth.

That makes the QEMU legacy serial ports enter the shared-IRQ THRE test path:

  serial8250_do_startup()
    if (port->irqflags & IRQF_SHARED)
      disable_irq_nosync(port->irq)
    ...
    if (port->irqflags & IRQF_SHARED)
      enable_irq(port->irq)

One possible interleaving is:

  CPU0, ttyS1                         CPU1, ttyS3

  serial_link_irq_chain()
    hash_add(i)
    i->head = &ttyS1
    request_irq()
                                        serial_link_irq_chain()
                                          find i in irq_lists
                                          list_add(&ttyS3, i->head)
                                        serial8250_do_startup()
                                          disable_irq_nosync(irq)
    irq_startup()
      desc->depth = 0
                                          enable_irq(irq)
                                            WARN: Unbalanced enable for IRQ 3

Keep hash_mutex held in serial_link_irq_chain() until the first request_irq()
has completed.  This prevents another 8250 port sharing the IRQ from joining
the chain and running the THRE test while the IRQ core is still starting the
interrupt.

This was reproduced in QEMU with ttyS1 and ttyS3 sharing IRQ 3.  With this
change, 100000 synchronized open/close iterations on /dev/ttyS1 and /dev/ttyS3
completed without the warning.

Fixes: 64c79dfbc458 ("serial: 8250_pnp: Support configurable reg shift property")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Cc: stable@vger.kernel.org # 6.10+
Assisted-by: Codex:gpt-5
Signed-off-by: Wang Zhaolong <wangzhaolong@fnnas.com>
---
 drivers/tty/serial/8250/8250_core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb..64eed4dc343f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -132,12 +132,10 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
  */
 static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
 {
 	struct irq_info *i;
 
-	guard(mutex)(&hash_mutex);
-
 	hash_for_each_possible(irq_lists, i, node, up->port.irq)
 		if (i->irq == up->port.irq)
 			return i;
 
 	i = kzalloc_obj(*i);
@@ -154,10 +152,12 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
 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);
 
 	scoped_guard(spinlock_irq, &i->lock) {
@@ -169,10 +169,15 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
 
 		INIT_LIST_HEAD(&up->list);
 		i->head = &up->list;
 	}
 
+	/*
+	 * Keep the shared-IRQ chain locked until the first handler is installed.
+	 * Otherwise another UART can join early and run startup IRQ masking while
+	 * the IRQ core is still enabling the line, unbalancing the disable depth.
+	 */
 	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;
-- 
2.54.0

Re: [PATCH] serial: 8250: serialize shared IRQ startup

[Wang Zhaolong]

On Wed, May 27, 2026 at 05:20:51PM +0800, Wang Zhaolong wrote:
> Concurrent startup of two 8250 ports sharing the same IRQ can trigger an
> IRQ core warning:
> 
>   Unbalanced enable for IRQ 3
>   WARNING: CPU: 0 PID: 580 at kernel/irq/manage.c:774 __enable_irq+0x3b/0x60
>   Call Trace:
>    enable_irq+0x8d/0x120
>    serial8250_do_startup+0x80d/0xa80
>    uart_port_startup+0x13d/0x440
>    uart_port_activate+0x5b/0xb0
>    tty_port_open+0xa1/0x120
>    uart_open+0x1e/0x30
>    tty_open+0x140/0x7a0
> 
> The second port can then run the shared-IRQ startup test while the IRQ core
> is still enabling the line for the first port.  The local
> disable_irq_nosync()/enable_irq() pair is balanced, but the interleaving can
> still unbalance the IRQ core disable depth.
> 
> That makes the QEMU legacy serial ports enter the shared-IRQ THRE test path:
> 
>   serial8250_do_startup()
>     if (port->irqflags & IRQF_SHARED)
>       disable_irq_nosync(port->irq)
>     ...
>     if (port->irqflags & IRQF_SHARED)
>       enable_irq(port->irq)
> 
> One possible interleaving is:
> 
>   CPU0, ttyS1                         CPU1, ttyS3
> 
>   serial_link_irq_chain()
>     hash_add(i)
>     i->head = &ttyS1
>     request_irq()
>                                         serial_link_irq_chain()
>                                           find i in irq_lists
>                                           list_add(&ttyS3, i->head)
>                                         serial8250_do_startup()
>                                           disable_irq_nosync(irq)
>     irq_startup()
>       desc->depth = 0
>                                           enable_irq(irq)
>                                             WARN: Unbalanced enable for IRQ 3
> 
> Keep hash_mutex held in serial_link_irq_chain() until the first request_irq()
> has completed.  This prevents another 8250 port sharing the IRQ from joining
> the chain and running the THRE test while the IRQ core is still starting the
> interrupt.
> 
> This was reproduced in QEMU with ttyS1 and ttyS3 sharing IRQ 3.  With this
> change, 100000 synchronized open/close iterations on /dev/ttyS1 and /dev/ttyS3
> completed without the warning.
> 
> Fixes: 64c79dfbc458 ("serial: 8250_pnp: Support configurable reg shift property")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
> Cc: stable@vger.kernel.org # 6.10+
> Assisted-by: Codex:gpt-5
> Signed-off-by: Wang Zhaolong <wangzhaolong@fnnas.com>
> ---
>  drivers/tty/serial/8250/8250_core.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index a428e88938eb..64eed4dc343f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -132,12 +132,10 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
>   */
>  static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
>  {
>  	struct irq_info *i;
>  
> -	guard(mutex)(&hash_mutex);
> -
>  	hash_for_each_possible(irq_lists, i, node, up->port.irq)
>  		if (i->irq == up->port.irq)
>  			return i;
>  
>  	i = kzalloc_obj(*i);
> @@ -154,10 +152,12 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
>  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);
>  
>  	scoped_guard(spinlock_irq, &i->lock) {
> @@ -169,10 +169,15 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
>  
>  		INIT_LIST_HEAD(&up->list);
>  		i->head = &up->list;
>  	}
>  
> +	/*
> +	 * Keep the shared-IRQ chain locked until the first handler is installed.
> +	 * Otherwise another UART can join early and run startup IRQ masking while
> +	 * the IRQ core is still enabling the line, unbalancing the disable depth.
> +	 */
>  	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;
> -- 
> 2.54.0

Hi Maintainers,

Friendly ping on this patch.

This is a clean and simple one-line relocation fix for the shared IRQ race condition.

I noticed there is another ongoing thread attempting to address the same bug with a
much more complex approach, but it seems to miss the regression test cases.

Could you please take a look at this simpler alternative when you have time? Any
feedback or reviews would be highly appreciated.

Thanks,
Wang

[PATCH v2] serial: 8250: fix shared IRQ startup race causing IRQ warning

[Wang Zhaolong]

Concurrent startup of two 8250 ports sharing the same IRQ can trigger an
IRQ core warning:

  Unbalanced enable for IRQ 3
  WARNING: CPU: 0 PID: 580 at kernel/irq/manage.c:774 __enable_irq+0x3b/0x60
  Call Trace:
   enable_irq+0x8d/0x120
   serial8250_do_startup+0x80d/0xa80
   uart_port_startup+0x13d/0x440
   uart_port_activate+0x5b/0xb0
   tty_port_open+0xa1/0x120
   uart_open+0x1e/0x30
   tty_open+0x140/0x7a0

This is reproducible in QEMU with four legacy 8250/16550 ports where ttyS1
and ttyS3 share IRQ 3.  A small userspace reproducer that synchronizes two
threads before open(), waits for both open attempts, and then closes both file
descriptors can trigger the warning almost immediately.

The regression was bisected to commit 64c79dfbc458 ("serial: 8250_pnp:
Support configurable reg shift property").  That change made QEMU's legacy
PNP serial ports take the shared-IRQ THRE test path in
serial8250_do_startup():

  if (port->irqflags & IRQF_SHARED)
    disable_irq_nosync(port->irq)
  ...
  if (port->irqflags & IRQF_SHARED)
    enable_irq(port->irq)

The disable_irq_nosync()/enable_irq() pair is locally balanced, but it can
race with the IRQ core startup path for the first 8250 port on the same IRQ.
One possible interleaving is:

  CPU0, ttyS1                         CPU1, ttyS3

  serial_link_irq_chain()
    hash_add(i)
    i->head = &ttyS1
    request_irq()
                                        serial_link_irq_chain()
                                          find i in irq_lists
                                          list_add(&ttyS3, i->head)
                                        serial8250_do_startup()
                                          disable_irq_nosync(irq)
    irq_startup()
      desc->depth = 0
                                          enable_irq(irq)
                                            WARN: Unbalanced enable for IRQ 3

Hold hash_mutex in serial_link_irq_chain() until the first request_irq() has
completed.  This prevents another 8250 port sharing the IRQ from joining the
chain and running the THRE test while the IRQ core is still starting the
interrupt.  The request_irq() failure cleanup also remains covered by
hash_mutex, so the just-published irq_info cannot be observed by another link
attempt before it is unlinked again.

Fixes: 64c79dfbc458 ("serial: 8250_pnp: Support configurable reg shift property")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Cc: stable@vger.kernel.org # 6.10+
Signed-off-by: Wang Zhaolong <wangzhaolong@fnnas.com>
---

Changes in v2:
  - Retitle the patch to describe the unbalanced IRQ enable warning.
  - Move the code comment to the hash_mutex acquisition site to document why the
    lock must cover the first request_irq() completion.
  - Drop the Assisted-by tag.

 drivers/tty/serial/8250/8250_core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb..dd202032cc7c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -132,12 +132,10 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
  */
 static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
 {
 	struct irq_info *i;
 
-	guard(mutex)(&hash_mutex);
-
 	hash_for_each_possible(irq_lists, i, node, up->port.irq)
 		if (i->irq == up->port.irq)
 			return i;
 
 	i = kzalloc_obj(*i);
@@ -154,10 +152,18 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
 static int serial_link_irq_chain(struct uart_8250_port *up)
 {
 	struct irq_info *i;
 	int ret;
 
+	/*
+	 * Keep the hash lock held until the first request_irq() completes.
+	 * The first port publishes i->head before request_irq() starts the IRQ;
+	 * a second port sharing the IRQ must not join the chain and run the
+	 * THRE test while the IRQ core is still bringing the line up.
+	 */
+	guard(mutex)(&hash_mutex);
+
 	i = serial_get_or_create_irq_info(up);
 	if (IS_ERR(i))
 		return PTR_ERR(i);
 
 	scoped_guard(spinlock_irq, &i->lock) {
-- 
2.54.0

Re: [PATCH v2] serial: 8250: fix shared IRQ startup race causing IRQ warning

[Jiri Slaby]

Ah, you are fixing the same thing as:
https://lore.kernel.org/all/20260707-bug-221579-8250-shared-irq-race-v6-1-f8c499a90bdd@gmail.com/

I did not look up who of you was first.

Note the above contains you in the commit log:
Reported-by: Wang Zhaolong <wangzhaolong@fnnas.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579

You both CCed each other, so you know the other's submission. I don't 
understand why you both send the patch? Anyway, you guys speak to each 
other and decide who sends the (fixed) patch.

On 08. 07. 26, 5:11, Wang Zhaolong wrote:
...
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -154,10 +152,18 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
>   static int serial_link_irq_chain(struct uart_8250_port *up)
>   {
>   	struct irq_info *i;
>   	int ret;
>   
> +	/*
> +	 * Keep the hash lock held until the first request_irq() completes.
> +	 * The first port publishes i->head before request_irq() starts the IRQ;
> +	 * a second port sharing the IRQ must not join the chain and run the
> +	 * THRE test while the IRQ core is still bringing the line up.
> +	 */
> +	guard(mutex)(&hash_mutex);

The same as in the other patch:
hash_mutex is no longer an appropriate name for this lock.

> +
>   	i = serial_get_or_create_irq_info(up);
>   	if (IS_ERR(i))
>   		return PTR_ERR(i);
>   
>   	scoped_guard(spinlock_irq, &i->lock) {

thanks,
-- 
js
suse labs

Re: [PATCH v2] serial: 8250: fix shared IRQ startup race causing IRQ warning

[Wang Zhaolong]

> From: "Jiri Slaby"<jirislaby@kernel.org>
> Date:  Wed, Jul 8, 2026, 2:03 PM
> Subject:  Re: [PATCH v2] serial: 8250: fix shared IRQ startup race causing IRQ warning
> To: "Wang Zhaolong"<wangzhaolong@fnnas.com>, <gregkh@linuxfoundation.org>
> Cc: <linux-serial@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>, <andriy.shevchenko@linux.intel.com>, <albanhuang@tencent.com>, <tombinfan@tencent.com>, <jackzxcui1989@163.com>, <kees@kernel.org>, <osama.abdelkader@gmail.com>, <realwujing@gmail.com>
> Ah, you are fixing the same thing as:
> https://lore.kernel.org/all/20260707-bug-221579-8250-shared-irq-race-v6-1-f8c499a90bdd@gmail.com/
> 
> I did not look up who of you was first.
> 
> Note the above contains you in the commit log:
> Reported-by: Wang Zhaolong <wangzhaolong@fnnas.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
> 
> You both CCed each other, so you know the other's submission. I don't 
> understand why you both send the patch? Anyway, you guys speak to each 
> other and decide who sends the (fixed) patch.
> 
> On 08. 07. 26, 5:11, Wang Zhaolong wrote:
> ...
> > --- a/drivers/tty/serial/8250/8250_core.c
> > +++ b/drivers/tty/serial/8250/8250_core.c
> > @@ -154,10 +152,18 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
> >   static int serial_link_irq_chain(struct uart_8250_port *up)
> >   {
> >           struct irq_info *i;
> >           int ret;
> >   
> > +        /*
> > +         * Keep the hash lock held until the first request_irq() completes.
> > +         * The first port publishes i->head before request_irq() starts the IRQ;
> > +         * a second port sharing the IRQ must not join the chain and run the
> > +         * THRE test while the IRQ core is still bringing the line up.
> > +         */
> > +        guard(mutex)(&hash_mutex);
> 
> The same as in the other patch:
> hash_mutex is no longer an appropriate name for this lock.
> 
> > +
> >           i = serial_get_or_create_irq_info(up);
> >           if (IS_ERR(i))
> >                   return PTR_ERR(i);
> >   
> >           scoped_guard(spinlock_irq, &i->lock) {
> 
> thanks,
> -- 
> js
> suse labs
> 

Hi Jiri,

Thanks for looking.

Just to clarify the timeline: I reported this issue and posted the original
minimal fix on May 27:

  https://lore.kernel.org/r/20260527092052.2086342-1-wangzhaolong@fnnas.com

I also pinged it on Jun 24:

  https://lore.kernel.org/r/ajtFoTHUQHJGYV5Q@MiniServer/

The other series was posted after my original submission and eventually
converged on the same core locking change.  I had also commented on that
series around v2/v3, pointing out the same startup race and the need to cover
the first request_irq() completion, so I was aware of that thread.

I sent v2 to refresh my earlier minimal fix with a clearer subject and commit
message, since the original patch had not received review.  My intent was not
to create two competing fixes for the same issue.

I agree with your comment that hash_mutex is no longer a great name once it
also serializes the first request_irq() completion.  I can send a v3 which
renames it and keeps the patch focused on the required locking change.

Thanks,
Wang Zhaolong

Re: [PATCH v2] serial: 8250: fix shared IRQ startup race causing IRQ warning

[Greg KH]

On Wed, Jul 08, 2026 at 02:20:34PM +0800, Wang Zhaolong wrote:
> 
> > From: "Jiri Slaby"<jirislaby@kernel.org>
> > Date:  Wed, Jul 8, 2026, 2:03 PM
> > Subject:  Re: [PATCH v2] serial: 8250: fix shared IRQ startup race causing IRQ warning
> > To: "Wang Zhaolong"<wangzhaolong@fnnas.com>, <gregkh@linuxfoundation.org>
> > Cc: <linux-serial@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>, <andriy.shevchenko@linux.intel.com>, <albanhuang@tencent.com>, <tombinfan@tencent.com>, <jackzxcui1989@163.com>, <kees@kernel.org>, <osama.abdelkader@gmail.com>, <realwujing@gmail.com>
> > Ah, you are fixing the same thing as:
> > https://lore.kernel.org/all/20260707-bug-221579-8250-shared-irq-race-v6-1-f8c499a90bdd@gmail.com/
> > 
> > I did not look up who of you was first.
> > 
> > Note the above contains you in the commit log:
> > Reported-by: Wang Zhaolong <wangzhaolong@fnnas.com>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
> > 
> > You both CCed each other, so you know the other's submission. I don't 
> > understand why you both send the patch? Anyway, you guys speak to each 
> > other and decide who sends the (fixed) patch.
> > 
> > On 08. 07. 26, 5:11, Wang Zhaolong wrote:
> > ...
> > > --- a/drivers/tty/serial/8250/8250_core.c
> > > +++ b/drivers/tty/serial/8250/8250_core.c
> > > @@ -154,10 +152,18 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
> > >   static int serial_link_irq_chain(struct uart_8250_port *up)
> > >   {
> > >           struct irq_info *i;
> > >           int ret;
> > >   
> > > +        /*
> > > +         * Keep the hash lock held until the first request_irq() completes.
> > > +         * The first port publishes i->head before request_irq() starts the IRQ;
> > > +         * a second port sharing the IRQ must not join the chain and run the
> > > +         * THRE test while the IRQ core is still bringing the line up.
> > > +         */
> > > +        guard(mutex)(&hash_mutex);
> > 
> > The same as in the other patch:
> > hash_mutex is no longer an appropriate name for this lock.
> > 
> > > +
> > >           i = serial_get_or_create_irq_info(up);
> > >           if (IS_ERR(i))
> > >                   return PTR_ERR(i);
> > >   
> > >           scoped_guard(spinlock_irq, &i->lock) {
> > 
> > thanks,
> > -- 
> > js
> > suse labs
> > 
> 
> Hi Jiri,
> 
> Thanks for looking.
> 
> Just to clarify the timeline: I reported this issue and posted the original
> minimal fix on May 27:
> 
>   https://lore.kernel.org/r/20260527092052.2086342-1-wangzhaolong@fnnas.com
> 
> I also pinged it on Jun 24:
> 
>   https://lore.kernel.org/r/ajtFoTHUQHJGYV5Q@MiniServer/
> 
> The other series was posted after my original submission and eventually
> converged on the same core locking change.  I had also commented on that
> series around v2/v3, pointing out the same startup race and the need to cover
> the first request_irq() completion, so I was aware of that thread.
> 
> I sent v2 to refresh my earlier minimal fix with a clearer subject and commit
> message, since the original patch had not received review.  My intent was not
> to create two competing fixes for the same issue.
> 
> I agree with your comment that hash_mutex is no longer a great name once it
> also serializes the first request_irq() completion.  I can send a v3 which
> renames it and keeps the patch focused on the required locking change.

Ok, I agree with Jiri here.  You two need to get together and agree on
what the final submission is, and how to properly attribute it.  Without
that, I'm not going to take either of these as it's obviously a mess.

Also, given that you both worked on this together (accidentally), odds
are it should just be a co-developed-by: tag being used.

thanks,

greg k-h

[PATCH v3] serial: 8250: fix shared IRQ startup race causing IRQ warning

[Wang Zhaolong]

Concurrent startup of two 8250 ports sharing the same IRQ can trigger an
IRQ core warning:

  Unbalanced enable for IRQ 3
  WARNING: CPU: 0 PID: 580 at kernel/irq/manage.c:774 __enable_irq+0x3b/0x60
  Call Trace:
   enable_irq+0x8d/0x120
   serial8250_do_startup+0x80d/0xa80
   uart_port_startup+0x13d/0x440
   uart_port_activate+0x5b/0xb0
   tty_port_open+0xa1/0x120
   uart_open+0x1e/0x30
   tty_open+0x140/0x7a0

This is reproducible in QEMU with four legacy 8250/16550 ports.  In that
setup, ttyS1 and ttyS3 share IRQ 3.  A small userspace reproducer
synchronizes two threads before open(), waits for both open attempts, and
then closes both file descriptors.  It can trigger the warning almost
immediately.

The regression was bisected to commit 64c79dfbc458 ("serial: 8250_pnp:
Support configurable reg shift property").  That change made QEMU's legacy
PNP serial ports take the shared-IRQ THRE test path in
serial8250_do_startup():

  if (port->irqflags & IRQF_SHARED)
    disable_irq_nosync(port->irq)
  ...
  if (port->irqflags & IRQF_SHARED)
    enable_irq(port->irq)

The disable_irq_nosync()/enable_irq() pair is locally balanced, but it can
race with the IRQ core startup path for the first 8250 port on the same
IRQ.  One possible interleaving is:

  CPU0, ttyS1                         CPU1, ttyS3

  serial_link_irq_chain()
    hash_add(i)
    i->head = &ttyS1
    request_irq()
                                        serial_link_irq_chain()
                                          find i in irq_lists
                                          list_add(&ttyS3, i->head)
                                        serial8250_do_startup()
                                          disable_irq_nosync(irq)
    irq_startup()
      desc->depth = 0
                                          enable_irq(irq)
                                            WARN: Unbalanced enable for IRQ 3

Hold irq_chain_mutex in serial_link_irq_chain() until the first request_irq()
has completed.  This prevents another 8250 port sharing the IRQ from joining
the chain and running the THRE test while the IRQ core is still starting the
interrupt.  The request_irq() failure cleanup also remains covered by
irq_chain_mutex, so the just-published irq_info cannot be observed by another
link attempt before it is unlinked again.

The lock used to only protect irq_lists hash walks, but it now also serializes
IRQ chain publication and the first request_irq() completion.  Rename it to
irq_chain_mutex and document the locking requirement for
serial_get_or_create_irq_info() with __must_hold() and lockdep_assert_held().

Fixes: 64c79dfbc458 ("serial: 8250_pnp: Support configurable reg shift property")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221579
Cc: stable@vger.kernel.org # 6.10+
Signed-off-by: Wang Zhaolong <wangzhaolong@fnnas.com>
---
Changes in v3:
  - Rename hash_mutex to irq_chain_mutex now that it also serializes IRQ chain
    publication and first request_irq() completion.
  - Add __must_hold() and lockdep_assert_held() to document the locking
    requirement for serial_get_or_create_irq_info().
  - Verify again with the QEMU ttyS1/ttyS3 shared IRQ reproducer.

Changes in v2:
  - Retitle the patch to describe the unbalanced IRQ enable warning.
  - Move the code comment to the mutex acquisition site to document why the
    lock must cover the first request_irq() completion.
  - Drop the Assisted-by tag.

v2: https://lore.kernel.org/all/20260708031115.3757150-1-wangzhaolong@fnnas.com/
v1: https://lore.kernel.org/all/20260527092052.2086342-1-wangzhaolong@fnnas.com/

 drivers/tty/serial/8250/8250_core.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index f49862d90eeb..41d87cdc69d7 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -48,11 +48,11 @@ struct irq_info {
 	struct list_head	*head;
 };
 
 #define IRQ_HASH_BITS		5	/* Can be adjusted later */
 static DEFINE_HASHTABLE(irq_lists, IRQ_HASH_BITS);
-static DEFINE_MUTEX(hash_mutex);	/* Used to walk the hash */
+static DEFINE_MUTEX(irq_chain_mutex);
 
 static bool skip_txen_test;
 module_param(skip_txen_test, bool, 0644);
 MODULE_PARM_DESC(skip_txen_test, "Skip checking for the TXEN bug at init time");
 
@@ -129,14 +129,15 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
  * Either:
  * - find the corresponding info in the hashtable and return it, or
  * - 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(&irq_chain_mutex)
 {
 	struct irq_info *i;
 
-	guard(mutex)(&hash_mutex);
+	lockdep_assert_held(&irq_chain_mutex);
 
 	hash_for_each_possible(irq_lists, i, node, up->port.irq)
 		if (i->irq == up->port.irq)
 			return i;
 
@@ -154,10 +155,18 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
 static int serial_link_irq_chain(struct uart_8250_port *up)
 {
 	struct irq_info *i;
 	int ret;
 
+	/*
+	 * Keep the IRQ chain lock held until the first request_irq() completes.
+	 * The first port publishes i->head before request_irq() starts the IRQ;
+	 * a second port sharing the IRQ must not join the chain and run the
+	 * THRE test while the IRQ core is still bringing the line up.
+	 */
+	guard(mutex)(&irq_chain_mutex);
+
 	i = serial_get_or_create_irq_info(up);
 	if (IS_ERR(i))
 		return PTR_ERR(i);
 
 	scoped_guard(spinlock_irq, &i->lock) {
@@ -180,11 +189,11 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
 
 static void serial_unlink_irq_chain(struct uart_8250_port *up)
 {
 	struct irq_info *i;
 
-	guard(mutex)(&hash_mutex);
+	guard(mutex)(&irq_chain_mutex);
 
 	hash_for_each_possible(irq_lists, i, node, up->port.irq)
 		if (i->irq == up->port.irq) {
 			if (WARN_ON(i->head == NULL))
 				return;
-- 
2.54.0

Re: [PATCH v3] serial: 8250: fix shared IRQ startup race causing IRQ warning

[Jing Wu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

On Wed, Jul 08, 2026 at 03:23:06PM +0800, Wang Zhaolong wrote:
> Changes in v3:
>   - Rename hash_mutex to irq_chain_mutex now that it also serializes IRQ chain
>     publication and first request_irq() completion.
>   - Add __must_hold() and lockdep_assert_held() to document the locking
>     requirement for serial_get_or_create_irq_info().

Wang, these changes listed in your v3 changelog were all made in our
series first — by Jiri's review feedback to *our* patches, not yours:

  - irq_chain_mutex rename: our v7, sent before your v3
  - __must_hold() + lockdep_assert_held(): our v5, sent on Jun 24

Your v3 copies these verbatim and presents them as your own work in the
changelog.  No Co-developed-by, no mention of our series at all.

The full timeline:

  May 27  Wang v1 — only the THRE test race, no use-after-free fix
  May 28  Our v1 — both races, full fix
  May 29  Wang on our v3 thread: "v3 fixes the Bugzilla reproducer"
  Jun 24  Our v5 — added __must_hold() and lockdep_assert_held()
  Jul  7  Our v6 — back to guard style
  Jul  8  Our v7 — renamed hash_mutex to irq_chain_mutex
  Jul  8  Wang v3 — copies irq_chain_mutex rename, __must_hold(),
                  lockdep_assert_held() from our v5/v7

We added Co-developed-by: Wang Zhaolong to our v7.  You've applied
several of our changes across your v2 and v3 without offering the same
credit.  Please add the appropriate Co-developed-by tags.

Jiri has asked us to coordinate.  We've been cooperative — we credited
you, we're running the same fix.  Let's work together on this.

Thanks,
Jing

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

[Jing Wu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]

> This accusation is not acceptable.

It's not an accusation.  It's a timeline.

Your v1 predates ours by one day — not in dispute.  But your v1 didn't
work.  You confirmed this yourself on May 29 when you wrote "v3 fixes
the Bugzilla reproducer on my setup" — on *our* v3 thread, not yours.
If your v1 already fixed the problem, you wouldn't have needed to test
our v3 and report back.

> That same locking change also closes the irq_info UAF window

Your v1 commit message never mentions use-after-free.  It discusses only
"Unbalanced enable for IRQ."  Our v1 was the first to identify and
document both races.  Claiming retroactively that your v1 "also" fixed
UAF is revisionist.

> For v3, the irq_chain_mutex rename was made in response to Jiri's review
> comment

Jiri made that comment on *our* v6, not yours.  He reviewed our code.
You took a review comment from our thread and applied it to your patch
as if it were your own insight.

> The __must_hold() and lockdep_assert_held() additions ... are not the
> functional fix.

They were added in our v5, reviewed by Greg on our v4.  You copied them
into your v3 and listed them as "Changes in v3" with zero attribution.

Now look at the timeline:

  Jul 7 22:06  Our v6 — posted to the list
  Jul 8 11:11  Your v2 — next morning, same lock-move skeleton
  Jul 8 15:23  Your v3 — hours later, copies irq_chain_mutex rename,
               __must_hold(), lockdep_assert_held() from our v5/v7

If we hadn't replied to Jiri and sent v6, would your v2 exist the next
morning?  If we hadn't renamed hash_mutex in v7, would your v3 appear
hours later with the same rename?  The answer is obvious.

> I do not agree to a Co-developed-by tag for my patch

We added Co-developed-by for you on our v7.  The only reason we did so
is that you confirmed the fix worked on your setup — nothing more.  You
reported the bug, you tested the fix, we credited you.  That's what
open source contributors do.

You, on the other hand, copied our lockdep annotations, our static
analysis annotations, our mutex rename — all reviewed by Jiri and Greg
on *our* patches — and passed them off as your own changelog entries.
And now you're saying you won't extend the same basic courtesy in return.
Copying is fine under GPLv2.  Presenting it as your own work is not.

You reported the bug.  We fixed it.  You copied our improvements.  Add
the tags.

Jing