linux-kernel - Re: [tip: irq/urgent] irqchip/gic-v3-its: Fix potential race condition in its_vlpi_prop

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 03 Jun 2024 17:01:10 +0100
From: Marc Zyngier <maz@...nel.org>
To: linux-kernel@...r.kernel.org
Cc: linux-tip-commits@...r.kernel.org,
	Hagar Hemdan <hagarhem@...zon.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	stable@...r.kernel.org,
	x86@...nel.org
Subject: Re: [tip: irq/urgent] irqchip/gic-v3-its: Fix potential race condition in its_vlpi_prop_update()

On Mon, 03 Jun 2024 13:25:06 +0100,
"tip-bot2 for Hagar Hemdan" <tip-bot2@...utronix.de> 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@...zon.com>
> AuthorDate:    Fri, 31 May 2024 16:21:44 
> Committer:     Thomas Gleixner <tglx@...utronix.de>
> 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@...nel.org>
> Signed-off-by: Hagar Hemdan <hagarhem@...zon.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Cc: stable@...r.kernel.org
> Reviewed-by: Marc Zyngier <maz@...nel.org>
> Link: https://lore.kernel.org/r/20240531162144.28650-1-hagarhem@amazon.com
> ---
>  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.