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

[Jiri Slaby <jirislaby@kernel.org>]

On 24. 06. 26, 10:43, Jing Wu wrote:
> From: Qiliang Yuan <realwujing@gmail.com>
> 
> 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