linux-kernel - Re: [RFC PATCH] x86/fpu/xstate: Add more diagnostic information on inconsistent xstate sizes

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, 10 Apr 2023 13:43:10 -0700
From:   Fenghua Yu <fenghua.yu@...el.com>
To:     "Chang S. Bae" <chang.seok.bae@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>
CC:     linux-kernel <linux-kernel@...r.kernel.org>, x86 <x86@...nel.org>,
        Chintan M Patel <chintan.m.patel@...el.com>,
        Thiago Macieira <thiago.macieira@...el.com>
Subject: Re: [RFC PATCH] x86/fpu/xstate: Add more diagnostic information on
 inconsistent xstate sizes

Hi, Chang,

On 4/7/23 11:22, Chang S. Bae wrote:
> On 4/5/2023 11:39 AM, Fenghua Yu wrote:
>> A warning is emitted when xstate sizes are found inconsistent.
>> But the warning message doesn't show enough information to diagnose
>> the issue.
>>
>> Provide more detailed xstate size information to help debug the issue.
>> As a hypothetical example, on a platform that may report incorrect
>> xstate size in CPUID.0xd.1:EBX, the diagnostic information after
>> the warning will show:
>> [    0.000000] x86/fpu: max_features=0x407
>> [    0.000000] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
>> [    0.000000] x86/fpu: xstate_offset[10]: 832, xstate_sizes[10]:    8
>> [    0.000000] x86/fpu: total size: 840 bytes
>> [    0.000000] x86/fpu: XCR0=0x7, IA32_XSS=0x400
>> [    0.000000] x86/fpu: kernel_size from CPUID.0xd.0x1:EBX: 576 bytes
>>
>> XCR0 | IA32_XSS is 0x407 which is consistent with max_features.
>> CPUID.0xd.0x1:EBX should report the size of the xsave area
>> containing xstate components corresponding to bits set in
>> XCR0 | IA32_XSS. But it only reports 576 bytes of xsave area which
>> doesn't count xstate sizes for AVX (offset 2 and 256 bytes) and
>> PASID (offset 10 and 8 bytes). This confirms that the platform
>> reports xstate size incorrectly through the CPUID bits.
>>
>> Suggest-by: Dave Hansen <dave.hansen@...ux.intel.com>
>> Tested-by: Chintan M Patel <chintan.m.patel@...el.com>
>> Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
>> ---
>>   arch/x86/kernel/fpu/xstate.c | 33 +++++++++++++++++++++++++++++++--
>>   1 file changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
>> index 0bab497c9436..5f27fcdc6c90 100644
>> --- a/arch/x86/kernel/fpu/xstate.c
>> +++ b/arch/x86/kernel/fpu/xstate.c
>> @@ -602,8 +602,37 @@ static bool __init 
>> paranoid_xstate_size_valid(unsigned int kernel_size)
>>           }
>>       }
>>       size = xstate_calculate_size(fpu_kernel_cfg.max_features, 
>> compacted);
>> -    XSTATE_WARN_ON(size != kernel_size,
>> -               "size %u != kernel_size %u\n", size, kernel_size);
>> +    if (size != kernel_size) {
>> +        u64 xcr0, ia32_xss;
>> +
>> +        XSTATE_WARN_ON(1, "size %u != kernel_size %u\n",
>> +                   size, kernel_size);
>> +
>> +        /* Show more information to help diagnose the size issue. */
>> +        pr_info("x86/fpu: max_features=0x%llx\n",
>> +            fpu_kernel_cfg.max_features);
>> +        print_xstate_offset_size();
>> +        pr_info("x86/fpu: total size: %u bytes\n", size);
>> +        xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
>> +        if (compacted) {
>> +            rdmsrl(MSR_IA32_XSS, ia32_xss);
> 
> This shouldn't be directly read here because of the LBR state component.
> 
> See the function comment:
> 
>   * Independent XSAVE features allocate their own buffers and are not
>   * covered by these checks. Only the size of the buffer for task->fpu
>   * is checked here.
> 
> But, isn't that max_features bitmask pretty much about it?

How about getting IA32_XSS from xfeatures_mask_supervisor()? That's how 
to get kernel_size by setting IA32_XSS without independent features in 
get_xsave_compacted_size().

Thanks.

-Fenghua