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

[Jiri Slaby]

On 24. 06. 26, 10:43, Jing Wu wrote:
> From: Qiliang Yuan <realwujing@xxxxxxxxx>
>
> On Wed, Jun 24, 2026 at 05:31:59AM +0200, Jiri Slaby wrote:
> > So what is the reason to switch from guards to manual locking?
>
> Scope-based guards release the lock at the end of the enclosing block,
> but the fix requires hash_mutex to be held across request_irq() and
> released at different exit points:
>
>    1. IS_ERR(i) -- release hash_mutex and return error.
>    2. Already in chain -- release i->lock, release hash_mutex, return 0.
>    3. First port, request_irq() fails -- cleanup under hash_mutex, then
>       release it and return error.
>    4. First port, request_irq() succeeds -- release hash_mutex, return 0.
>
> These paths span different nesting levels and early returns, so scope
> guards cannot express the required lock lifecycle.  The same applies to
> i->lock: it must be dropped before calling request_irq() (cannot hold a
> spinlock while sleeping), but hash_mutex must remain held across the
> call, which also breaks the guard model.

I don't follow, the function now looks like this:

        mutex_lock(&hash_mutex);

        i = serial_get_or_create_irq_info(up);
        if (IS_ERR(i)) {
                mutex_unlock(&hash_mutex);
                return PTR_ERR(i);
        }

        /*
...
         */
        spin_lock_irq(&i->lock);
        if (i->head) {
                list_add(&up->list, i->head);
                spin_unlock_irq(&i->lock);
                mutex_unlock(&hash_mutex);

                return 0;
        }

        INIT_LIST_HEAD(&up->list);
        i->head = &up->list;
        spin_unlock_irq(&i->lock);

        ret = request_irq(up->port.irq, serial8250_interrupt,
                          up->port.irqflags, up->port.name, i);
        if (ret < 0) {
                serial_do_unlink(i, up);
                mutex_unlock(&hash_mutex);
                return ret;
        }

        mutex_unlock(&hash_mutex);

        return 0;

What am I missing?

thanks,
--
js
suse labs