[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 16 Aug 2023 17:55:09 +0100
From: Andrew Cooper <andrew.cooper3@...rix.com>
To: Solar Designer <solar@...nwall.com>
Cc: "Xen. org security team" <security@....org>,
oss-security@...ts.openwall.com
Subject: Re: Xen Security Advisory 433 v3 (CVE-2023-20593) -
x86/AMD: Zenbleed
On 16/08/2023 5:41 pm, Solar Designer wrote:
> On Tue, Aug 08, 2023 at 07:18:51PM +0100, Andrew Cooper wrote:
>> On 08/08/2023 7:00 pm, Solar Designer wrote:
>>> + /*
>>> + * Microcode is the preferred mitigation, in terms of performance.
>>> + * However, without microcode, this chickenbit (specific to the Zen2
>>> + * uarch) disables Floating Point Mov-Elimination to mitigate the
>>> + * issue.
>>> + */
>>> + val &= ~chickenbit;
>>> + if (sig->rev < good_rev)
>>> + val |= chickenbit;
>>>
>>> This leaves me wondering: why have this line at all? I understand Xen
>>> wanting to enable the chicken bit on vulnerable CPUs, but why disable it
>>> on other AMD CPUs? If someone or something had enabled the bit, that's
>>> probably intentional, and even if not it probably shouldn't be Xen's
>>> business to alter CPU behavior beyond what's necessary for Xen itself to
>>> work reliably and securely.
>>>
>>> Am I missing something?
>> There is an earlier exit in this function for any non-Zen2 system.
>>
>> So here, we are strictly on Zen2 (all vulnerable), and either have good
>> microcode or not.
>>
>> The microcode fix is far more performant than the chickenbit.
> Sure, but that's orthogonal to my concern, which was about areas of
> responsibility and control (such as sysadmin vs. tools).
>
> Anyway, it was pointed out to me off-list that Linux kernel does the
> same thing, also explicitly disabling chickenbit when deemed safe:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=522b1d69219d8f083173819fde04f994aa051a98
>
> + if (!cpu_has_zenbleed_microcode()) {
> + pr_notice_once("Zenbleed: please update your microcode for the most optimal fix\n");
> + msr_set_bit(MSR_AMD64_DE_CFG, MSR_AMD64_DE_CFG_ZEN2_FP_BACKUP_FIX_BIT);
> + } else {
> + msr_clear_bit(MSR_AMD64_DE_CFG, MSR_AMD64_DE_CFG_ZEN2_FP_BACKUP_FIX_BIT);
> + }
>
> So at least it's a consistent approach by these two projects, and a
> reason for Xen to be doing it this way.
It is not a coincidence that Xen and Linux are similar here. The areas
of responsibility aspect was raised during review - we did consider
combining with the old value.
AMD's position AIUI is that prior to Zenbleed, this bit was unsupported
and not used. Therefore we went for the simpler approach (and as you
saw, still managed to screw that up).
~Andrew
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
