linux-kernel - Re: [PATCH v2 12/21] KVM:x86: Add fault checks for guest CR4.CET setting

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:   Fri, 5 May 2023 15:24:40 +0800
From:   "Yang, Weijiang" <weijiang.yang@...el.com>
To:     Binbin Wu <binbin.wu@...ux.intel.com>
CC:     <rick.p.edgecombe@...el.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        <seanjc@...gle.com>, <pbonzini@...hat.com>, <peterz@...radead.org>,
        <john.allen@....com>, <kvm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 12/21] KVM:x86: Add fault checks for guest CR4.CET
 setting


On 5/5/2023 1:01 PM, Binbin Wu wrote:
>
>
> On 4/21/2023 9:46 PM, Yang Weijiang wrote: 
[...]
>> @@ -995,6 +995,9 @@ int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned 
>> long cr0)
>>           (is_64_bit_mode(vcpu) || kvm_is_cr4_bit_set(vcpu, 
>> X86_CR4_PCIDE)))
>>           return 1;
>>   +    if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET))
> You can use kvm_is_cr4_bit_set() instead of kvm_read_cr4_bits()

Good suggestion, thanks!

>
>> +        return 1;
>> +
>>       static_call(kvm_x86_set_cr0)(vcpu, cr0);
>>         kvm_post_set_cr0(vcpu, old_cr0, cr0);
>> @@ -1210,6 +1213,9 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned 
>> long cr4)
>>               return 1;
>>       }
>>   +    if ((cr4 & X86_CR4_CET) && !(kvm_read_cr0(vcpu) & X86_CR0_WP))
> You can use kvm_is_cr0_bit_set() to check X86_CR0_WP

OK.

>
>> +        return 1;
>> +
>>
[...]
>> @@ -536,6 +536,9 @@ bool kvm_msr_allowed(struct kvm_vcpu *vcpu, u32 
>> index, u32 type);
>>           __reserved_bits |= X86_CR4_VMXE;        \
>>       if (!__cpu_has(__c, X86_FEATURE_PCID))          \
>>           __reserved_bits |= X86_CR4_PCIDE;       \
>> +    if (!__cpu_has(__c, X86_FEATURE_SHSTK) &&    \
>> +        !__cpu_has(__c, X86_FEATURE_IBT))        \
>> +        __reserved_bits |= X86_CR4_CET;        \
> IMO, it is a bit wired to split this part from the change of 
> CR4_RESERVED_BITS.

Make sense, will move these lines to other patch.

>
>
>> __reserved_bits;                                \
>>   })
>