Re: [PATCH v2] irqchip: crossbar: Fix data race in allocate_gic_irq

[Bhargav Joshi]

Hi,

On Thu, Jun 11, 2026 at 8:13 PM Thomas Gleixner <tglx@xxxxxxxxxx> wrote:
>
> On Wed, Jun 10 2026 at 16:44, Bhargav Joshi wrote:
>
> Subject prefix is: irqchip/crossbar: .....
>
> allocate_git_irq is a function and wants to be denoted as such. See
>
>   https://docs.kernel.org/process/maintainer-tip.html
>
> > In allocate_gic_irq(), if irq_domain_alloc_irqs_parent() fails, the
> > error path resets cb->irq_map[i] to IRQ_FREE. It modifies cb->irq_map[]
> > without holding cb->lock. modifying without lock could cause data race.
>
> That's not a useful explanation. You fail to tell what races against it
> and what the side effects of a racy access are.
>
> > Fix this by acquiring raw_spin_lock around cb->irq_map[] modification.
> >
> > Fixes: 783d31863fb8 ("irqchip: crossbar: Convert dra7 crossbar to stacked domains")
> >
>
> Pointless newline.
>
> > Signed-off-by: Bhargav Joshi <j.bhargav.u@xxxxxxxxx>
> > ---
> > This bug was flagged by the Sashiko AI bot during the review process for
> > the DT schema conversion of ti,irq-crossbar binding.
> > https://lore.kernel.org/linux-devicetree/20260605210647.CCC881F00893@xxxxxxxxxxxxxxx/
>
> Interesting. Because Sashiko said so, there is a bug, right?
No, my initial analysis considered two concurrent allocate_gic_irq()
calls one failing irq_domain_alloc_irqs_parent() and writing
cb->irq_map[i] = IRQ_FREE without the lock, while the other is scanning
the array inside the lock in loop. That would be an unlocked write
concurrent with a locked read.
But I missed that the IRQ domain core serializes .alloc()/.free().
Apologies for that. I will drop this patch.

Best Regards,
Bhargav