linux-kernel - Re: [PATCH 1/2] x86/mce: Disable preemption for CPER decoding

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:   Thu, 22 Jun 2023 15:23:49 -0400
From:   Yazen Ghannam <yazen.ghannam@....com>
To:     "Luck, Tony" <tony.luck@...el.com>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>
Cc:     yazen.ghannam@....com,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH 1/2] x86/mce: Disable preemption for CPER decoding

On 6/22/2023 1:05 PM, Luck, Tony wrote:
>> This is the reason we search for the logical CPU number using the Local
>> APIC ID provided in the CPER. And fill in relevant data using that CPU
>> number.
> 
> So you don't care which CPU number mce_setup() used because you are
> going to update it with the right one from CPER?
>

That's right.

> Then maybe the fix for part 1 is just to use raw_smp_processor_id() instead of
> smp_processor_id() to avoid the warning for calling with pre-emption enabled,
> instead of disabling premption with the get_cpu() ... put_cpu() wrap around the
> call to mce_setup()?

You mean use raw_smp_processor_id() in mce_setup()? I thought about 
that, but decided against it. I figure the preemption warning is helpful 
to catch issues when mce_setup() *is* supposed to run on the current CPU 
but doesn't.

This BERT decoding path is the only exception AFAIK. So I didn't want to 
change the common code for a single exception.

I just noticed a similar potential issue with mce_setup() in 
apei_mce_report_mem_error(). How is the CPU number decided there? Is it 
always "don't care", since the mce record is "fake"?

Here are another couple of solutions for the preemption issue.
1) Don't use mce_setup() at all. Instead, do the memset(), etc. in the 
local function. This would result in some code duplication.
2) Split mce_setup() into global and per_cpu parts. The memset(), cpuid, 
etc. would be global, and the cpu_data()* and rdmsr() would be per_cpu.

Option #2 can also be used in apei_mce_report_mem_error(), I think.

Thanks,
Yazen