linux-kernel - Re: [PATCH] x86/apic: reset LDR in clear_local

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]

Date:   Mon, 19 Aug 2019 18:05:50 -0400
From:   Bandan Das <bsd@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/apic: reset LDR in clear_local_APIC

Hi Thomas,

Thomas Gleixner <tglx@...utronix.de> writes:

> Bandan,
>
> On Wed, 14 Aug 2019, Bandan Das wrote:
>> On a 32 bit RHEL6 guest with greater than 8 cpus, the
>> kdump kernel hangs when calibrating apic. This happens
>> because when apic initializes bigsmp, it also initializes LDR
>> even though it probably wouldn't be used.
>
> 'It probably wouldn't be used' is a not really a useful technical
> statement.
>
> Either it is used, then it needs to be handled. Or it is unused then why is
> it written in the first place?
>
> The bigsmp APIC uses physical destination mode because the logical flat
> model only supports 8 APICs. So clearly bigsmp_init_apic_ldr() is bogus and
> should be empty.
>

Your note above is what I meant by "it probably wouldn't be used" because
I don't have much insight into the history behind why LDR is being initialized
in the first place. The only evidence I found is a comment in apic.c that states:
	/*
	 * Intel recommends to set DFR, LDR and TPR before enabling
	 * an APIC.  See e.g. "AP-388 82489DX User's Manual" (Intel
	 * document number 292116).  So here it goes...
	 */
That said, not initalizing the ldr in bigsmp_init_apic_ldr() should be
enough to fix this. Would you prefer that change instead ?

>> When booting into kdump, KVM apic incorrectly reads the stale LDR
>> values from the guest while building the logical destination map
>> even for inactive vcpus. While KVM apic can be fixed to ignore apics
>> that haven't been enabled, a simple guest only change can be to
>> just clear out the LDR.
>
> This does not make much sense either. What has KVM to do with logical
> destination maps while booting the kdump kernel? The kdump kernel is not

This is the guest kernel and KVM takes care of injecting the interrupt to
the right vcpu (recalculate_apic_map)() in lapic.c). 

For the KVM side change, please take a look at
https://lore.kernel.org/kvm/aee50952-144d-78da-9036-045bd3838b59@redhat.com/

> going through the regular cold/warm boot process, so KVM does not even know
> that the crashing kernel jumped into the kdump one.
>
> What builds the logical destination maps and what has LDR and the KVM APIC
> to do with that?
>
> I'm not opposed to the change per se, but I'm not accepting change logs
> which have the fairy tale smell.
>
Heh, no it's not.

> Thanks,
>
> 	tglx