linux-kernel - Re: [PATCH v2 2/3] xen/events: Return -EEXIST for bound VIRQs

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

Message-ID: <12dbe129-9132-4c8c-8002-5c34ce90d42d@amd.com>
Date: Wed, 27 Aug 2025 18:11:23 -0400
From: Jason Andryuk <jason.andryuk@....com>
To: Jürgen Groß <jgross@...e.com>, Stefano Stabellini
	<sstabellini@...nel.org>, Oleksandr Tyshchenko
	<oleksandr_tyshchenko@...m.com>
CC: <xen-devel@...ts.xenproject.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/3] xen/events: Return -EEXIST for bound VIRQs

On 2025-08-27 11:21, Jürgen Groß wrote:
> On 26.08.25 02:55, Jason Andryuk wrote:
>> Change find_virq() to return -EEXIST when a VIRQ is bound to a
>> different CPU than the one passed in.  With that, remove the BUG_ON()
>> from bind_virq_to_irq() to propogate the error upwards.
>>
>> Some VIRQs are per-cpu, but others are per-domain or global.  Those must
>> be bound to CPU0 and can then migrate elsewhere.  The lookup for
>> per-domain and global will probably fail when migrated off CPU 0,
>> especially when the current CPU is tracked.  This now returns -EEXIST
>> instead of BUG_ON().
>>
>> A second call to bind a per-domain or global VIRQ is not expected, but
>> make it non-fatal to avoid trying to look up the irq, since we don't
>> know which per_cpu(virq_to_irq) it will be in.
>>
>> Signed-off-by: Jason Andryuk <jason.andryuk@....com>
>
>> @@ -1381,8 +1387,9 @@ int bind_virq_to_irq(unsigned int virq, unsigned 
>> int cpu, bool percpu)
>>               evtchn = bind_virq.port;
>>           else {
>>               if (ret == -EEXIST)
>> -                ret = find_virq(virq, cpu, &evtchn);
>> -            BUG_ON(ret < 0);
>> +                ret = find_virq(virq, cpu, &evtchn, percpu);
>> +            if (ret)
>> +                goto out;
> 
> I think you are leaking info here. I guess a call of __unbind_from_irq() is
> wanted like in the error case below (note that the case of no valid 
> evtchn is
> handled there just fine).

Ok, thanks for catching that.

I'm going to add Cc: stable to the next version of this.  While it 
doesn't have a Fixes associated, we want this as a prerequisite for patch 3.

Regards,
Jason