linux-kernel - Re: [PATCH 08/67] KVM: x86: Pass new routing entries and irqfd when updating IRTEs

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: <d8fca516-235a-4156-aaf6-8cf41c47d0b0@amd.com>
Date: Fri, 11 Apr 2025 22:52:14 +0530
From: Sairaj Kodilkar <sarunkod@....com>
To: Sean Christopherson <seanjc@...gle.com>
CC: Paolo Bonzini <pbonzini@...hat.com>, Joerg Roedel <joro@...tes.org>,
	"David Woodhouse" <dwmw2@...radead.org>, Lu Baolu <baolu.lu@...ux.intel.com>,
	<kvm@...r.kernel.org>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, Maxim Levitsky <mlevitsk@...hat.com>, "Joao
 Martins" <joao.m.martins@...cle.com>, David Matlack <dmatlack@...gle.com>,
	Naveen N Rao <naveen.rao@....com>, Vasant Hegde <vasant.hegde@....com>
Subject: Re: [PATCH 08/67] KVM: x86: Pass new routing entries and irqfd when
 updating IRTEs



On 4/11/2025 7:31 PM, Sean Christopherson wrote:
> On Fri, Apr 11, 2025, Arun Kodilkar, Sairaj wrote:
>> On 4/5/2025 1:08 AM, Sean Christopherson wrote:
>>> +int avic_pi_update_irte(struct kvm_kernel_irqfd *irqfd, struct kvm *kvm,
>>> +			unsigned int host_irq, uint32_t guest_irq,
>>> +			struct kvm_kernel_irq_routing_entry *new)
>>>    {
>>>    	struct kvm_kernel_irq_routing_entry *e;
>>>    	struct kvm_irq_routing_table *irq_rt;
>>>    	bool enable_remapped_mode = true;
>>> +	bool set = !!new;
>>>    	int idx, ret = 0;
>>>    	if (!kvm_arch_has_assigned_device(kvm) || !kvm_arch_has_irq_bypass())
>>> @@ -925,6 +919,8 @@ int avic_pi_update_irte(struct kvm *kvm, unsigned int host_irq,
>>>    		if (e->type != KVM_IRQ_ROUTING_MSI)
>>>    			continue;
>>> +		WARN_ON_ONCE(new && memcmp(e, new, sizeof(*new)));
>>> +
>>>
>>
>> Hi Sean,
>>
>> In kvm_irq_routing_update() function, its possible that there are
>> multiple entries in the `kvm_irq_routing_table`,
> 
> Not if one of them is an MSI.  In setup_routing_entry():
> 
> 	/*
> 	 * Do not allow GSI to be mapped to the same irqchip more than once.
> 	 * Allow only one to one mapping between GSI and non-irqchip routing.
> 	 */
> 	hlist_for_each_entry(ei, &rt->map[gsi], link)
> 		if (ei->type != KVM_IRQ_ROUTING_IRQCHIP ||
> 		    ue->type != KVM_IRQ_ROUTING_IRQCHIP ||
> 		    ue->u.irqchip.irqchip == ei->irqchip.irqchip)
> 			return -EINVAL;
> 
>> and `irqfd_update()` ends up setting up the new entry type to 0 instead of
>> copying the entry.
>>
>> if (n_entries == 1)
>>      irqfd->irq_entry = *e;
>> else
>>      irqfd->irq_entry.type = 0;
>>
>> Since irqfd_update() did not copy the entry to irqfd->entries, the "new"
>> will not match entry "e" obtained from irq_rt, which can trigger a false
>> WARN_ON.
> 
> And since there can only be one MSI, if there are multiple routing entries, then
> the WARN won't be reached thanks to the continue that's just above:
> 
> 		if (e->type != KVM_IRQ_ROUTING_MSI)
> 			continue;

Thanks.. I understand it now. I did not see complete code hence the 
confusion. Sorry about that.

Regards
Sairaj kodilkar