[PATCH RFC] irqchip: crossbar: Fix data race in allocate_gic_irq

[PATCH RFC] irqchip: crossbar: Fix data race in allocate_gic_irq

[Bhargav Joshi]

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.

Fix this by acquiring raw_spin_lock around cb->irq_map[] modification.

Fixes: 783d31863fb8 ("irqchip: crossbar: Convert dra7 crossbar to stacked domains")

Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
---
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@smtp.kernel.org/
---
 drivers/irqchip/irq-crossbar.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index cd1134101ace..9b809e711009 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -100,8 +100,11 @@ static int allocate_gic_irq(struct irq_domain *domain, unsigned virq,
 	fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
 
 	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
-	if (err)
+	if (err) {
+		raw_spin_lock(&cb->lock);
 		cb->irq_map[i] = IRQ_FREE;
+		raw_spin_lock(&cb->lock);
+	}
 	else
 		cb->write(i, hwirq);
 

---
base-commit: 2d3090a8aeb596a26935db0955d46c9a5db5c6ce
change-id: 20260610-irq-spinlock-fix-1c90d8bc0f13

Best regards,
--  
Bhargav

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

[Marc Zyngier]

On Tue, 09 Jun 2026 22:00:09 +0100,
Bhargav Joshi <j.bhargav.u@gmail.com> wrote:
> 
> 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.
> 
> Fix this by acquiring raw_spin_lock around cb->irq_map[] modification.
> 
> Fixes: 783d31863fb8 ("irqchip: crossbar: Convert dra7 crossbar to stacked domains")
> 
> Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
> ---
> 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@smtp.kernel.org/
> ---
>  drivers/irqchip/irq-crossbar.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
> index cd1134101ace..9b809e711009 100644
> --- a/drivers/irqchip/irq-crossbar.c
> +++ b/drivers/irqchip/irq-crossbar.c
> @@ -100,8 +100,11 @@ static int allocate_gic_irq(struct irq_domain *domain, unsigned virq,
>  	fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
>  
>  	err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
> -	if (err)
> +	if (err) {
> +		raw_spin_lock(&cb->lock);
>  		cb->irq_map[i] = IRQ_FREE;
> +		raw_spin_lock(&cb->lock);

Really?

	M.

-- 
Without deviation from the norm, progress is not possible.

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

[Bhargav Joshi]

On Wed, Jun 10, 2026 at 12:21 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 09 Jun 2026 22:00:09 +0100,
> Bhargav Joshi <j.bhargav.u@gmail.com> wrote:
> >
> > 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.
> >
> > Fix this by acquiring raw_spin_lock around cb->irq_map[] modification.
> >
> > Fixes: 783d31863fb8 ("irqchip: crossbar: Convert dra7 crossbar to stacked domains")
> >
> > Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
> > ---
> > 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@smtp.kernel.org/
> > ---
> >  drivers/irqchip/irq-crossbar.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
> > index cd1134101ace..9b809e711009 100644
> > --- a/drivers/irqchip/irq-crossbar.c
> > +++ b/drivers/irqchip/irq-crossbar.c
> > @@ -100,8 +100,11 @@ static int allocate_gic_irq(struct irq_domain *domain, unsigned virq,
> >       fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
> >
> >       err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
> > -     if (err)
> > +     if (err) {
> > +             raw_spin_lock(&cb->lock);
> >               cb->irq_map[i] = IRQ_FREE;
> > +             raw_spin_lock(&cb->lock);
>
> Really?
>
Apologies for the typo, Should I send a v2 with the proper
raw_spin_unlock(), or do you feel this specific error path doesn't
warrant the fix?

>         M.
>
> --
> Without deviation from the norm, progress is not possible.

Best Regards,
Bhargav

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

[Marc Zyngier]

On Wed, 10 Jun 2026 08:20:59 +0100,
Bhargav Joshi <j.bhargav.u@gmail.com> wrote:
> 
> On Wed, Jun 10, 2026 at 12:21 PM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Tue, 09 Jun 2026 22:00:09 +0100,
> > Bhargav Joshi <j.bhargav.u@gmail.com> wrote:
> > >
> > > 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.
> > >
> > > Fix this by acquiring raw_spin_lock around cb->irq_map[] modification.
> > >
> > > Fixes: 783d31863fb8 ("irqchip: crossbar: Convert dra7 crossbar to stacked domains")
> > >
> > > Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
> > > ---
> > > 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@smtp.kernel.org/
> > > ---
> > >  drivers/irqchip/irq-crossbar.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
> > > index cd1134101ace..9b809e711009 100644
> > > --- a/drivers/irqchip/irq-crossbar.c
> > > +++ b/drivers/irqchip/irq-crossbar.c
> > > @@ -100,8 +100,11 @@ static int allocate_gic_irq(struct irq_domain *domain, unsigned virq,
> > >       fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
> > >
> > >       err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
> > > -     if (err)
> > > +     if (err) {
> > > +             raw_spin_lock(&cb->lock);
> > >               cb->irq_map[i] = IRQ_FREE;
> > > +             raw_spin_lock(&cb->lock);
> >
> > Really?
> >
> Apologies for the typo, Should I send a v2 with the proper
> raw_spin_unlock(), or do you feel this specific error path doesn't
> warrant the fix?

I don't know what to answer to that question. Surely *you* have done
the necessary analysis before sending a patch, right?

	M.

-- 
Without deviation from the norm, progress is not possible.

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

[Bhargav Joshi]

On Wed, Jun 10, 2026 at 2:03 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Wed, 10 Jun 2026 08:20:59 +0100,
> Bhargav Joshi <j.bhargav.u@gmail.com> wrote:
> >
> > On Wed, Jun 10, 2026 at 12:21 PM Marc Zyngier <maz@kernel.org> wrote:
> > >
> > > On Tue, 09 Jun 2026 22:00:09 +0100,
> > > Bhargav Joshi <j.bhargav.u@gmail.com> wrote:
> > > >
> > > > 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.
> > > >
> > > > Fix this by acquiring raw_spin_lock around cb->irq_map[] modification.
> > > >
> > > > Fixes: 783d31863fb8 ("irqchip: crossbar: Convert dra7 crossbar to stacked domains")
> > > >
> > > > Signed-off-by: Bhargav Joshi <j.bhargav.u@gmail.com>
> > > > ---
> > > > 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@smtp.kernel.org/
> > > > ---
> > > >  drivers/irqchip/irq-crossbar.c | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
> > > > index cd1134101ace..9b809e711009 100644
> > > > --- a/drivers/irqchip/irq-crossbar.c
> > > > +++ b/drivers/irqchip/irq-crossbar.c
> > > > @@ -100,8 +100,11 @@ static int allocate_gic_irq(struct irq_domain *domain, unsigned virq,
> > > >       fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
> > > >
> > > >       err = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
> > > > -     if (err)
> > > > +     if (err) {
> > > > +             raw_spin_lock(&cb->lock);
> > > >               cb->irq_map[i] = IRQ_FREE;
> > > > +             raw_spin_lock(&cb->lock);
> > >
> > > Really?
> > >
> > Apologies for the typo, Should I send a v2 with the proper
> > raw_spin_unlock(), or do you feel this specific error path doesn't
> > warrant the fix?
>
> I don't know what to answer to that question. Surely *you* have done
> the necessary analysis before sending a patch, right?
Yes, modifying cb->irq_map without the lock can create a data race. If
irq_domain_alloc_irqs_parent() fails it could cause an unlocked write
that is concurrent with locked reads. I'll send the v2 with the
corrected raw_spin_unlock.

>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

Best Regards,
Bhargav