linux-kernel - [POSSIBLE BUG] behavior change in irq_can_handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAMOZA0+TMC8FsFfboe5RKqYNRxYm=cg602p5FXAwpyA64Y4yHA@mail.gmail.com>
Date: Sat, 8 Nov 2025 22:30:18 +0100
From: Luigi Rizzo <lrizzo@...gle.com>
To: linux-kernel@...r.kernel.org, maz@...nel.org, tglx@...utronix.de
Cc: Luigi Rizzo <rizzo.unipi@...il.com>, Sean Christopherson <seanjc@...gle.com>, 
	Bjorn Helgaas <bhelgaas@...gle.com>
Subject: [POSSIBLE BUG] behavior change in irq_can_handle_pm() introduced in 8d39d6ec4db5d

BACKGROUND (just to explain how I found the issue; it may exist regardless):

I have some code (soon to be posted here) to implement interrupt moderation
in software using using per-CPU hrtimers. The basic logic is the following:

- if the system decides an irq needs moderation, it calls disable_irq_nosync(),
  adds the irq_desc in a per-cpu list, and keeps IRQD_IRQ_INPROGRESS set
  to prevent migration. The first desc inserted in the list also start
an hrtimer;

- when the timer fires, the callback clears the bit and calls enable_irq()
  on all linked irq_desc's

The relevant code is the following:

@@ -207,x +208,x @@ irqreturn_t handle_irq_event(struct irq_desc *desc)

        raw_spin_lock(&desc->lock);
+       /* if moderation kicks in, disable_irq_nosync() and set an
hrtimer. Keep the bit set to prevent migration */
+       if (irq_moderation_has_started_timer_and_disabled_irq(desc))
+               return ret;
        irqd_clear(&desc->irq_data, IRQD_IRQ_INPROGRESS);
        return ret;

and the timer callback does the following:

+       list_for_each_entry_safe(desc, next, &ms->descs, ms_node) {
+               list_del(&desc->ms_node);
+               INIT_LIST_HEAD(&desc->ms_node);
+               /* To prevent migration, IRQD_IRQ_INPROGRESS was left
on. Clear it now. */
+               raw_spin_lock(&desc->lock); /* maybe not necessary ? */
+               irqd_clear(&desc->irq_data, IRQD_IRQ_INPROGRESS);
+               raw_spin_unlock(&desc->lock);
+               enable_irq(desc->irq_data.irq); /* valid if the chip
sync_lock/unlock are null */
+       }

ISSUE:
After the following change enable_irq() almost immediately causes a deadlock
(it used to work before):

8d39d6ec4db5d genirq: Prevent migration live lock in handle_edge_irq()
 kernel/irq/chip.c | 41 +++++++++++++++++++++++++++++++++++++++--

likely because the extra checks in irq_can_handle_pm(), specifically
the chunk below:

@@ -501,x +504,x @@ static bool irq_can_handle_pm(struct irq_desc *desc)
                        return false;
                return irq_wait_on_inprogress(desc);
        }
-       return false;
...
+        * If the interrupt affinity was moved to this CPU and the
+        * interrupt is currently handled on the previous target CPU, then
+        * busy wait for INPROGRESS to be cleared.
...
+       aff = irq_data_get_effective_affinity_mask(irqd);
+       if (cpumask_first(aff) != smp_processor_id())    <==== is this correct ?
+               return false;
+       return irq_wait_on_inprogress(desc);

QUESTION:
I am not sure the condition indicated by the arrow matches the comment.
Can someone clarify ?

Otherwise, for my use case, I have successfully tested the following change,
which reverts to the previous behavior at least for my specific use case:

@@ -501,x +504,x @@ static bool irq_can_handle_pm(struct irq_desc *desc)
                        return false;
                return irq_wait_on_inprogress(desc);
        }
+       if (irqd_irq_disabled(irqd))
+               return false;

thanks
luigi