On 11/3/20 2:28 PM, Kirill Tkhai wrote:
> On 02.11.2020 20:13, Andrey Ryabinin wrote:
>> If several threads read /proc/cpuinfo some can see in 'flags'
>> values from c->x86_capability, before __do_cpuid_fault() called
>> and masks applied. Fix this by forming 'flags' on stack first
>> and copy them in per_cpu(cpu_flags, cpu) as a last step.
>>
>> https://jira.sw.ru/browse/PSBM-121823
>> Signed-off-by: Andrey Ryabinin <aryabi...@virtuozzo.com>
>> ---
>> Changes since v1:
>> - none
>>
>> arch/x86/kernel/cpu/proc.c | 17 +++++++++--------
>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>> index 4fe1577d5e6f..4cc2951e34fb 100644
>> --- a/arch/x86/kernel/cpu/proc.c
>> +++ b/arch/x86/kernel/cpu/proc.c
>> @@ -69,11 +69,11 @@ static DEFINE_PER_CPU(struct cpu_flags, cpu_flags);
>> static void init_cpu_flags(void *dummy)
>> {
>> int cpu = smp_processor_id();
>> - struct cpu_flags *flags = &per_cpu(cpu_flags, cpu);
>> + struct cpu_flags flags;
>> struct cpuinfo_x86 *c = &cpu_data(cpu);
>> unsigned int eax, ebx, ecx, edx;
>>
>> - memcpy(flags->val, c->x86_capability, NCAPINTS * sizeof(u32));
>> + memcpy(&flags, c->x86_capability, sizeof(flags));
>>
>> /*
>> * Clear feature bits masked using cpuid masking/faulting.
>> @@ -81,26 +81,27 @@ static void init_cpu_flags(void *dummy)
>>
>> if (c->cpuid_level >= 0x00000001) {
>> __do_cpuid_fault(0x00000001, 0, &eax, &ebx, &ecx, &edx);
>> - flags->val[4] &= ecx;
>> - flags->val[0] &= edx;
>> + flags.val[4] &= ecx;
>> + flags.val[0] &= edx;
>> }
>>
>> if (c->cpuid_level >= 0x00000007) {
>> __do_cpuid_fault(0x00000007, 0, &eax, &ebx, &ecx, &edx);
>> - flags->val[9] &= ebx;
>> + flags.val[9] &= ebx;
>> }
>>
>> if ((c->extended_cpuid_level & 0xffff0000) == 0x80000000 &&
>> c->extended_cpuid_level >= 0x80000001) {
>> __do_cpuid_fault(0x80000001, 0, &eax, &ebx, &ecx, &edx);
>> - flags->val[6] &= ecx;
>> - flags->val[1] &= edx;
>> + flags.val[6] &= ecx;
>> + flags.val[1] &= edx;
>> }
>>
>> if (c->cpuid_level >= 0x0000000d) {
>> __do_cpuid_fault(0x0000000d, 1, &eax, &ebx, &ecx, &edx);
>> - flags->val[10] &= eax;
>> + flags.val[10] &= eax;
>> }
>> + memcpy(&per_cpu(cpu_flags, cpu), &flags, sizeof(flags));
>
> This is still racy, since memcpy() is not atomic. Maybe we should add some
> lock on top of this?
>
This race shouldn't be a problem since flags are not supposed to change during
ve lifetime.
So we overwriting same values. But don't mind to add spinlock protection.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel