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: <Z_kgbna7grb833Fy@google.com>
Date: Fri, 11 Apr 2025 07:01:29 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Sairaj Arun Kodilkar <sarunkod@....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 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;