linux-kernel - Re: [PATCH] irqchip/gic-v3-its: fix raw_local_irq

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]

Message-ID: <81111C9C-2A06-4865-8A3C-1DF10F174910@amazon.de>
Date: Mon, 30 Dec 2024 15:10:20 +0000
From: "Krcka, Tomas" <krckatom@...zon.de>
To: Marc Zyngier <maz@...nel.org>
CC: "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, Thomas Gleixner <tglx@...utronix.de>,
	Hagar Hemdan <hagarhem@...zon.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] irqchip/gic-v3-its: fix raw_local_irq_restore() called with IRQs
 enabled



> On 30. Dec 2024, at 15:28, Marc Zyngier <maz@...nel.org> wrote:
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> Hi Tomas,
> 
> On Mon, 30 Dec 2024 13:49:03 +0000,
> Tomas Krcka <tomas.krcka@...il.com> wrote:
>> 
>> The following call-chain leads to misuse of spinlock_irq
>> when spinlock_irqsave was hold.
>> 
>> irq_set_vcpu_affinity
>>  -> irq_get_desc_lock (spinlock_irqsave)
>>   -> its_irq_set_vcpu_affinity
>>    -> guard(raw_spin_lock_irq) <--- this enables interrupts
>>  -> irq_put_desc_unlock // <--- WARN IRQs enabled
> 
> Urgh. Nice catch. It really should have been either raw_spin_lock or
> the _irqsave variant, but not the _irq variant which should really
> never be used outside of the core code. I guess this was never really
> tested when rewritten at commit time...
> 
>> Fix the issue by using guard(raw_spinlock), since the function is
>> already called with irqsave and raw_spin_lock was used before the commit
>> b97e8a2f7130 ("irqchip/gic-v3-its: Fix potential race condition in its_vlpi_prop_update()")
>> introducing the guard as well.
>> 
>> This was discovered through the lock debugging, and the corresponding
>> log is as follows:
>> 
>> raw_local_irq_restore() called with IRQs enabled
>> WARNING: CPU: 38 PID: 444 at kernel/locking/irqflag-debug.c:10 warn_bogus_irq_restore+0x2c/0x38
>> Call trace:
>>  warn_bogus_irq_restore+0x2c/0x38
>>   _raw_spin_unlock_irqrestore+0x68/0x88
>>   __irq_put_desc_unlock+0x1c/0x48
>>   irq_set_vcpu_affinity+0x74/0xc0
>>   its_map_vlpi+0x44/0x88
>>   kvm_vgic_v4_set_forwarding+0x148/0x230
>>   kvm_arch_irq_bypass_add_producer+0x20/0x28
>>   __connect+0x98/0xb8
>>   irq_bypass_register_consumer+0x150/0x178
>>   kvm_irqfd+0x6dc/0x744
>>   kvm_vm_ioctl+0xe44/0x16b0
>> 
>> Fixes: b97e8a2f7130 ("irqchip/gic-v3-its: Fix potential race condition in its_vlpi_prop_update()")
>> Signed-off-by: Tomas Krcka <krckatom@...zon.de>
> 
> Two problems here:
> 
> - there is no "From Tomas Krcka <krckatom@...zon.de>" at the beginning
>  of the patch, which is needed since you are posting from a gmail.com
>  address
> 
> - there is no SoB using your gmail.com address, which is needed since
>  this patch appears to be from your Amazon doppelganger.
> 
> So either you post this patch from your amazon.de email, or you add
> the two missing pieces of information that are required.
> 
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 92244cfa0464..8c3ec5734f1e 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -2045,7 +2045,7 @@ 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);
>> +     guard(raw_spinlock)(&its_dev->event_map.vlpi_lock);
> 
> This otherwise looks good to me. Please repost this with the above
> issues fixed, and the following tags:

Ack, reposted as v2 - https://lore.kernel.org/all/20241230150825.62894-1-krckatom@amazon.de/


> 
> Reviewed-by: Marc Zyngier <maz@...nel.org>
> Cc: stable@...r.kernel.org
> 
> Thanks,
> 
>        M.
> 
> --
> Without deviation from the norm, progress is not possible.





Amazon Web Services Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597