Re: [PATCH v5 1/3] serial: 8250: hold hash_mutex across IRQ chain linking in serial_link_irq_chain()

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3563 bytes --]

On Wed, 30 Sep 2026, Hui Peng wrote:

> In serial_link_irq_chain(), serial_get_or_create_irq_info() acquires
> hash_mutex to search or allocate a chain instance and drops hash_mutex
> before returning.
> 
> If a concurrent call to serial_unlink_irq_chain() runs immediately after
> serial_get_or_create_irq_info() returns while i->head is still NULL,
> serial_unlink_irq_chain() acquires hash_mutex, finds i with
> i->head == NULL, frees the chain structure via free_irq() / kfree(),
> and removes it from the hashtable. serial_link_irq_chain() then
> proceeds to scoped_guard(spinlock_irq, &i->lock), dereferencing the freed
> chain pointer i and causing a slab use-after-free.
> 
> Move guard(mutex)(&hash_mutex) to the beginning of
> serial_link_irq_chain() and rename serial_get_or_create_irq_info() to
> serial_get_or_create_irq_info_locked() so hash_mutex is held across the
> entire lookup, linking, and error-path unlinking sequence.
> 
> Tested in QEMU against tip of mainline (62f4c998b297, "Merge tag
> 'parisc-for-7.3-rc5' of git://git.kernel.org/.../parisc-linux")
> with KASAN enabled by concurrently opening/closing 8250 serial ports
> sharing IRQ 4 (ttyS0 and ttyS2): on the unfixed kernel KASAN detects
> slab-use-after-free in serial_link_irq_chain(); whereas with this fix
> applied, port registration and unregistration run cleanly with 0 KASAN
> warnings.
> 
> Fixes: e91f1ed6c24f ("serial: 8250: fix use-after-free in serial_link_irq_chain")
> Cc: stable@vger.kernel.org
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

I'm also pretty sure I've not given my tag for this so why did you add it?
Never invent Reviewed-by tags on your own.

--
 i.

> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> Changes in v5:
> - Rebased cleanly onto tip of mainline commit 62f4c998b297.
> - Updated patch diff and commit description to align with upstream
>   guard(mutex) refactoring in 8250_core.c.
> 
> Changes in v4:
> - No changes.
> 
> Changes in v3:
> - No changes.
> 
> Changes in v2:
> - Split out from monolithic patch into patch 1/3, add Cc: stable@vger.kernel.org,
>   and document QEMU test procedure in commit message body, as requested by
>   Greg Kroah-Hartman and John Ogness.
> 
>  drivers/tty/serial/8250/8250_core.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index b875d394796f..e831a8fe9a95 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -130,12 +130,10 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
>   * - 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)
> +static struct irq_info *serial_get_or_create_irq_info_locked(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;
> @@ -156,7 +154,9 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
>  	struct irq_info *i;
>  	int ret;
> 
> -	i = serial_get_or_create_irq_info(up);
> +	guard(mutex)(&hash_mutex);
> +
> +	i = serial_get_or_create_irq_info_locked(up);
>  	if (IS_ERR(i))
>  		return PTR_ERR(i);
>