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: <4c1a3a82-b443-ecd7-dc38-ac0574db0b50@intel.com>
Date:   Thu, 12 Jan 2023 09:56:22 -0800
From:   "Chen, Yian" <yian.chen@...el.com>
To:     Sohil Mehta <sohil.mehta@...el.com>,
        <linux-kernel@...r.kernel.org>, <x86@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ravi Shankar <ravi.v.shankar@...el.com>,
        "Tony Luck" <tony.luck@...el.com>, Paul Lai <paul.c.lai@...el.com>,
        Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Subject: Re: [PATCH 5/7] x86/cpu: Enable LASS (Linear Address Space
 Separation)



On 1/11/2023 2:22 PM, Sohil Mehta wrote:
>> +static __always_inline void setup_lass(struct cpuinfo_x86 *c)
>> +{
>> +    if (cpu_feature_enabled(X86_FEATURE_LASS)) {
>> +        cr4_set_bits(X86_CR4_LASS);
>> +    } else {
>> +        /*
>> +         * only clear the feature and cr4 bits when hardware
>> +         * supports LASS, in case it was enabled in a previous
>> +         * boot (e.g., via kexec)
>> +         */
>> +        if (cpu_has(c, X86_FEATURE_LASS)) {
>> +            cr4_clear_bits(X86_CR4_LASS);
>> +            clear_cpu_cap(c, X86_FEATURE_LASS);
>> +        }
>> +    }
>> +}
> 
> I am quite confused by the "else" code flow. Can you please help 
> understand how this code path would be exercised?
>
The "else" branch is to disable LASS for every cpu. Addition to clear 
the CR4 bit, it would be better to clear cpu feature id for consistent 
result in every CPU as well, otherwise,  we could see the feature id is 
cleared in the boot-cpu only, for example, if user enables vsyscall.

> Also, why don't other features such as SMAP or SMEP need this type of 
> handling? 
It seems there is no need to have such if-else for SMAP/SMEP since the 
X86_SMAP/SMEP was gone from arch/x86/Kconfig.

I see something on similar lines for UMIP.
> 
> Also, how does the CR4 pinning code in the following patch play into 
> this?
I did not test if pinning code works as commented:  "/* These bits 
should not change their value after CPU init is finished. */"

Could it flag a warning when cr4_clear_bits() is called above?
I did not observed a warning for cr4_clear_bits(), this should be okay 
since it is called during init.
> 



>> +
>>   /* These bits should not change their value after CPU init is 
>> finished. */
>>   static const unsigned long cr4_pinned_mask =
>>       X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_UMIP |
>> @@ -1848,6 +1865,7 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>>       setup_smep(c);
>>       setup_smap(c);
>>       setup_umip(c);
>> +    setup_lass(c);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ