Re: [tip: irq/urgent] irqchip/gic-v3-its: Fix potential race condition in its_vlpi_prop_update()

[Marc Zyngier]

On Mon, 03 Jun 2024 13:25:06 +0100,
"tip-bot2 for Hagar Hemdan" <tip-bot2@xxxxxxxxxxxxx> wrote:
> 
> The following commit has been merged into the irq/urgent branch of tip:
> 
> Commit-ID:     8dd4302d37bb2fe842acb3be688d393254b4f126
> Gitweb:        https://git.kernel.org/tip/8dd4302d37bb2fe842acb3be688d393254b4f126
> Author:        Hagar Hemdan <hagarhem@xxxxxxxxxx>
> AuthorDate:    Fri, 31 May 2024 16:21:44 
> Committer:     Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> CommitterDate: Mon, 03 Jun 2024 14:19:42 +02:00
> 
> irqchip/gic-v3-its: Fix potential race condition in its_vlpi_prop_update()
> 
> its_vlpi_prop_update() calls lpi_write_config() which obtains the
> mapping information for a VLPI without lock held. So it could race
> with its_vlpi_unmap().
> 
> Since all calls from its_irq_set_vcpu_affinity() require the same                                                                                                                                                                                                                                                            
> lock to be held, hoist the locking there instead of sprinkling the
> locking all over the place.
> 
> This bug was discovered using Coverity Static Analysis Security Testing
> (SAST) by Synopsys, Inc.
> 
> [ tglx: Use guard() instead of goto ]

Good call. Except that...

> 
> Fixes: 015ec0386ab6 ("irqchip/gic-v3-its: Add VLPI configuration handling")
> Suggested-by: Marc Zyngier <maz@xxxxxxxxxx>
> Signed-off-by: Hagar Hemdan <hagarhem@xxxxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Reviewed-by: Marc Zyngier <maz@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/20240531162144.28650-1-hagarhem@xxxxxxxxxx
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 44 ++++++++-----------------------
>  1 file changed, 12 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 40ebf17..c696ac9 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c

[...]

> @@ -1992,6 +1970,8 @@ static int its_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
>  	if (!is_v4(its_dev->its))
>  		return -EINVAL;
>  
> +	guard(raw_spinlock_irq, &its_dev->event_map.vlpi_lock);
> +

I don't think this compiles as is, due to the funky syntax required.

Thanks,

	M.

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