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

Date:   Wed, 21 Aug 2019 15:54:34 -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

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

> Bandan,
>
> On Wed, 21 Aug 2019, Bandan Das wrote:
>> Thomas Gleixner <tglx@...utronix.de> writes:
>> So, in KVM: if we make sure that the logical destination map isn't filled up if the virtual
>> apic is not enabled by software, it really doesn't matter whether the LDR for an inactive CPU
>> has a stale value.
>>
>> In x86/apic: if we make sure that the LDR is 0 or reset,
>> recalculate_apic_map() will never consider including this cpu in the
>> logical map.
> ?
>> In short, as I mentioned in the patch description, this is really a KVM
>> bug but it doesn't hurt to clear out the LDR in the guest and then, it
>> wouldn't need a hypervisor fix.
>
> I still needs a hypervisor fix. Taking disabled APICs into account is a bug
> which has also other consequeces than that particular one. So please don't
> claim that. It's wrong.
>
> If that prevents the APIC bug from triggering on unfixed hypervisors, then
> this is a nice side effect, but not a solution.
>
Agreed and fwiw, the kvm fix has been queued already.

>> Is this better ?
>
> That's way better.
>
> So can you please create two patches:
>
>    1) Make that bogus bigsmp ldr init empty
>
>       That one wants a changelog along these lines:
>
>       - Setting LDR for physical destination mode is pointless
>       - Setting multiple bits in the LDR is wrong
>
>       Mention how this was discovered and caused the KVM APIC bug to be
>       triggered. Also mention that the change is not there to paper over
>       the KVM APIC bug. The change fixes a bug in the bigsmp APIC code.
>
>    2) Clear LDR in in that apic reset function
>
>       That one wants a changelog along these lines:
>
>       - Except for x2apic the LDR should be cleared as any other APIC
>       	register
>
>       Mention how this was discovered. Again the change is not there to
>       paper over the KVM APIC bug. It's for correctness sake and valid on
>       its own.
>
> Thanks,
>
Will do as you suggested. Thank you for the review.

Bandan
> 	tglx
>
>