linux-kernel - Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking

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: <5407877d-4c7c-494f-8fc1-d44eea4762b9@citrix.com>
Date: Tue, 12 Aug 2025 20:10:09 +0100
From: Andrew Cooper <andrew.cooper3@...rix.com>
To: Jason Andryuk <jason.andryuk@....com>, Juergen Gross <jgross@...e.com>,
 Stefano Stabellini <sstabellini@...nel.org>,
 Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
 Chris Wright <chrisw@...s-sol.org>,
 Jeremy Fitzhardinge <jeremy@...source.com>,
 Christopher Clark <christopher.w.clark@...il.com>,
 Daniel Smith <dpsmith@...rtussolutions.com>
Cc: stable@...r.kernel.org, xen-devel@...ts.xenproject.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking

On 12/08/2025 8:00 pm, Jason Andryuk wrote:
> VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
> tracking of VIRQs is handled by per-cpu variables virq_to_irq.
>
> The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
> registration time - typically CPU 0.  Later, the interrupt can migrate,
> and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
> virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is
> called again with CPU 0, the stale irq is returned.
>
> Change the virq_to_irq tracking to use CPU 0 for per-domain and global
> VIRQs.  As there can be at most one of each, there is no need for
> per-vcpu tracking.  Also, per-domain and global VIRQs need to be
> registered on CPU 0 and can later move, so this matches the expectation.
>
> Fixes: e46cdb66c8fc ("xen: event channels")
> Cc: stable@...r.kernel.org
> Signed-off-by: Jason Andryuk <jason.andryuk@....com>
> ---
> Fixes is the introduction of the virq_to_irq per-cpu array.
>
> This was found with the out-of-tree argo driver during suspend/resume.
> On suspend, the per-domain VIRQ_ARGO is unbound.  On resume, the driver
> attempts to bind VIRQ_ARGO.  The stale irq is returned, but the
> WARN_ON(info == NULL || info->type != IRQT_VIRQ) in bind_virq_to_irq()
> triggers for NULL info.  The bind fails and execution continues with the
> driver trying to clean up by unbinding.  This eventually faults over the
> NULL info.

I don't think the Fixes: tag is entirely appropriate.

per-domain VIRQs were created (unexpectedly) by the merge of ARGO into
Xen.  It was during some unrelated cleanup that this was noticed and
bugfixed into working.  i.e. the ARGO VIRQ is the singular weird one here.

In Xen we did accept that per-domain VIRQs now exist; they had for
several releases before we realised.

~Andrew

> ---
>  drivers/xen/events/events_base.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 41309d38f78c..a27e4d7f061e 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -159,7 +159,19 @@ static DEFINE_MUTEX(irq_mapping_update_lock);
>  
>  static LIST_HEAD(xen_irq_list_head);
>  
> -/* IRQ <-> VIRQ mapping. */
> +static bool is_per_vcpu_virq(int virq) {
> +	switch (virq) {
> +	case VIRQ_TIMER:
> +	case VIRQ_DEBUG:
> +	case VIRQ_XENOPROF:
> +	case VIRQ_XENPMU:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +/* IRQ <-> VIRQ mapping.  Global/Domain virqs are tracked in cpu 0.  */
>  static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
>  
>  /* IRQ <-> IPI mapping */
> @@ -974,6 +986,9 @@ static void __unbind_from_irq(struct irq_info *info, unsigned int irq)
>  
>  		switch (info->type) {
>  		case IRQT_VIRQ:
> +			if (!is_per_vcpu_virq(virq_from_irq(info)))
> +				cpu = 0;
> +
>  			per_cpu(virq_to_irq, cpu)[virq_from_irq(info)] = -1;
>  			break;
>  		case IRQT_IPI: