linux-kernel - Re: [PATCH v5 13/19] x86/cpufeatures: Add flag to track whether MSR IA32_FEAT

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: <df215c4c-82f0-5b15-57c3-d304fd94ff3b@intel.com>
Date:   Wed, 26 Feb 2020 12:41:09 -0800
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     TonyWWang-oc@...oxin.com, acme@...nel.org,
        alexander.shishkin@...ux.intel.com, bp@...en8.de, bp@...e.de,
        hpa@...or.com, jacob.jun.pan@...ux.intel.com,
        jarkko.sakkinen@...ux.intel.com, jmattson@...gle.com,
        jolsa@...hat.com, joro@...tes.org, kvm@...r.kernel.org,
        lenb@...nel.org, linux-edac@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-pm@...r.kernel.org, mark.rutland@....com, mingo@...hat.com,
        namhyung@...nel.org, pbonzini@...hat.com, peterz@...radead.org,
        rkrcmar@...hat.com, shuah@...nel.org, tglx@...utronix.de,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        x86@...nel.org
Subject: Re: [PATCH v5 13/19] x86/cpufeatures: Add flag to track whether MSR
 IA32_FEAT_CTL is configured

On 2/25/2020 4:58 PM, Jacob Keller wrote:
> On 2/25/2020 4:42 PM, Sean Christopherson wrote>> So there's something
> weird going on. Maybe "boot_cpu_has" in the
>>> vmx_disabled_by_bios is wrong? Hmm.
>>
>> Hmm, perhaps a bug somewhere else is overwriting the cpufeatures bit for
>> X86_FEATURE_VMX.  Let me see if I can reproduce from net-next.
>>
> 
> If you have any further suggestions for debugging, I'm happy to help try
> to figure this out. To my eyes, it looks like somehow bits get reset...
> It's definitely not clear to me how this happens.
> 
> There is the get_cpu_caps call.. but that seems to correctly call
> apply_forced_caps at the end.
> 
> That's all I have time for today.
> 
> Thanks,
> Jake
> 

Hi,

I kept digging into this, and I added a further print to the get_cpu_cap
function.

It looks like get_cpu_cap is being called again *after*
init_ia32_feat_ctl...

Digging further, I discovered this appears to be the call in setup_pku,
which would only be enabled for systems which have X86_FEATURE_PKU
enabled and supported. It's quite likely that test systems may not have
had this feature, hence why it went undetected till now.

Because of the extra get_cpu_cap call, the capabilities are reset. Since
we never use setup_clear_cpu_cap or pass NULL to clear_cpu_cap, the code
that sets the global cpu_caps_cleared bits is not run.

It's not clear to me what the best fix for this is.

Perhaps init_ia32_feat_ctl should be something run during
early_identify_cpu, since it's really checking global status (rdmsr),
and not per-CPU status. And then it could directly operate to call
setup_clear_cpu_cap, which would properly clear the bit globally,
ensuring that apply_forced_caps kicks in?

Or this needs to somehow be run *after* setup_pku? But that doesn't feel
very robust.

Thanks,
Jake