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: <1918c63f-0065-b468-3691-de6e314f4374@suse.com>
Date:   Thu, 3 Nov 2022 14:38:07 +0100
From:   Jan Beulich <jbeulich@...e.com>
To:     Jane Malalane <jane.malalane@...rix.com>
Cc:     Juergen Gross <jgross@...e.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
        Maximilian Heyne <mheyne@...zon.de>,
        xen-devel@...ts.xenproject.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4] x86/xen: Add support for
 HVMOP_set_evtchn_upcall_vector

On 29.07.2022 09:04, Jane Malalane wrote:
> @@ -125,6 +130,9 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_xen_hvm_callback)
>  {
>  	struct pt_regs *old_regs = set_irq_regs(regs);
>  
> +	if (xen_percpu_upcall)
> +		ack_APIC_irq();
> +
>  	inc_irq_stat(irq_hv_callback_count);
>  
>  	xen_hvm_evtchn_do_upcall();
> @@ -168,6 +176,15 @@ static int xen_cpu_up_prepare_hvm(unsigned int cpu)
>  	if (!xen_have_vector_callback)
>  		return 0;
>  
> +	if (xen_percpu_upcall) {
> +		rc = xen_set_upcall_vector(cpu);

>From all I can tell at least for APs this happens before setup_local_apic().
With there being APIC interaction in this operation mode, as seen e.g. in
the earlier hunk above, I think this is logically wrong. And it leads to
apic_pending_intr_clear() issuing its warning: The vector registration, as
an intentional side effect, marks the vector as pending. Unless IRQs were
enabled at any point between the registration and the check, there's
simply no way for the corresponding IRR bit to be dealt with (by
propagating to ISR when the interrupt is delivered, and then being cleared
from ISR by EOI).

As a note to x86 maintainers: This comment there

	/*
	 * If the ISR map is not empty. ACK the APIC and run another round
	 * to verify whether a pending IRR has been unblocked and turned
	 * into a ISR.
	 */

is pretty clearly wrong - with IRQs disabled there's no way for a pending
IRR bit to be "unblocked and turned into a ISR" one. And even if IRQs
were enabled TPR would still prevent the handling of any bits potentially
set in the 16....31 range.

Jan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ