[PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()

[PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()

[Jiaqing Zhao]

When suspending, save_processor_state() calls mtrr_save_fixed_ranges()
to save fixed-range MTRRs. On platforms without fixed-range MTRRs,
accessing these MSRs will trigger unchecked MSR access error. Make
sure fixed-range MTRRs are supported before access to prevent such
error.

Since mtrr_state.have_fixed is only set when MTRRs are present and
enabled, checking the CPU feature flag in mtrr_save_fixed_ranges()
is unnecessary.

Fixes: 3ebad5905609 ("[PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending")
Cc: stable@vger.kernel.org
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
---
v2:
* Removed unnecessary boot_cpu_has(X86_FEATURE_MTRR) check.
* Updated commit message.
Link: https://lore.kernel.org/all/20250509085612.2236222-2-jiaqing.zhao@linux.intel.com

 arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index e2c6b471d230..8c18327eb10b 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -593,7 +593,7 @@ static void get_fixed_ranges(mtrr_type *frs)
 
 void mtrr_save_fixed_ranges(void *info)
 {
-	if (boot_cpu_has(X86_FEATURE_MTRR))
+	if (mtrr_state.have_fixed)
 		get_fixed_ranges(mtrr_state.fixed_ranges);
 }
 
-- 
2.43.0

Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()

[Borislav Petkov]

On Fri, May 09, 2025 at 05:06:33PM +0000, Jiaqing Zhao wrote:
> When suspending, save_processor_state() calls mtrr_save_fixed_ranges()
> to save fixed-range MTRRs. On platforms without fixed-range MTRRs,
> accessing these MSRs will trigger unchecked MSR access error. Make
> sure fixed-range MTRRs are supported before access to prevent such
> error.
> 
> Since mtrr_state.have_fixed is only set when MTRRs are present and
> enabled, checking the CPU feature flag in mtrr_save_fixed_ranges()
> is unnecessary.
> 
> Fixes: 3ebad5905609 ("[PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>

Next question: this is CC:stable, meaning it'll go to Linus now.

What exactly is it fixing?

Because the patch in Fixes: is from 2007. :-\

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()

[Jiaqing Zhao]

On 2025-05-10 01:32, Borislav Petkov wrote:
> On Fri, May 09, 2025 at 05:06:33PM +0000, Jiaqing Zhao wrote:
>> When suspending, save_processor_state() calls mtrr_save_fixed_ranges()
>> to save fixed-range MTRRs. On platforms without fixed-range MTRRs,
>> accessing these MSRs will trigger unchecked MSR access error. Make
>> sure fixed-range MTRRs are supported before access to prevent such
>> error.
>>
>> Since mtrr_state.have_fixed is only set when MTRRs are present and
>> enabled, checking the CPU feature flag in mtrr_save_fixed_ranges()
>> is unnecessary.
>>
>> Fixes: 3ebad5905609 ("[PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
> 
> Next question: this is CC:stable, meaning it'll go to Linus now.
> 
> What exactly is it fixing?
> 
> Because the patch in Fixes: is from 2007. :-\

Hi, Boris

This fixes unchecked MSR access error on platform without fixed-range
MTRRs when doing ACPI S3 suspend. IMHO, though it is handled and won't
panic kernel, it is worth getting fixed, and it matches the stable rule
that

"It fixes a problem like an oops, a hang, data corruption, a real
security issue, a hardware quirk, a build error (but not for things
marked CONFIG_BROKEN), or some “oh, that’s not good” issue."

Kernel log is attached below.

Thanks,
Jiaqing

[ 173.115706] ACPI: PM: Saving platform NVS memory
[ 173.115818] Disabling non-boot CPUs ...
[ 173.126530] unchecked MSR access error: RDMSR from 0x250 at rIP: 0xffffffffa90a15ff (get_fixed_ranges+0x)
[ 173.126749] Call Trace:
[ 173.126806] <TASK>
[ 173.126858] ? show_stack_regs+0x23/0x30
[ 173.126946] ? fixup_exception+0x5a4/0x610
[ 173.127037] ? printk_get_next_message+0x105/0x350
[ 173.127141] ? gp_try_fixup_and_notify+0x37/0x100
[ 173.127244] ? exc_general_protection+0xe1/0x1f0
[ 173.127346] ? asm_exc_general_protection+0x27/0x30
[ 173.127452] ? __cfi_x86_acpi_suspend_lowlevel+0x10/0x10
[ 173.127567] ? get_fixed_ranges+0x5f/0x390
[ 173.127657] mtrr_save_fixed_ranges+0x1b/0x40
[ 173.127753] save_processor_state+0x111/0x220
[ 173.127849] do_suspend_lowlevel+0xf/0xb70
[ 173.127939] x86_acpi_suspend_lowlevel+0x14c/0x180
[ 173.128042] acpi_suspend_enter+0x17e/0x1e0
[ 173.128133] suspend_devices_and_enter+0x62d/0x950
[ 173.128236] pm_suspend+0x2cf/0x4c0
[ 173.128314] state_store+0x109/0x130
[ 173.128393] kobj_attr_store+0x1e/0x40
[ 173.128477] sysfs_kf_write+0x45/0x60
[ 173.128559] kernfs_fop_write_iter+0x113/0x1a0
[ 173.128698] vfs_write+0x38a/0x470
[ 173.128775] ksys_write+0x87/0x100
[ 173.128851] __x64_sys_write+0x1b/0x30
[ 173.128932] x64_sys_call+0x17f1/0x25e0
[ 173.129017] do_syscall_64+0x74/0x120
[ 173.129098] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 173.129206] RIP: 0033:0x7d6a19d82687
[ 173.129288] Code: 00 00 00 b8 00 00 00 00 0f 05 48 3d 01 f0 ff ff 72 09 f7 d8 89 c7 e8 e8 fa ff ff c3 0f0
[ 173.129661] RSP: 002b:00007d66c7a7f668 EFLAGS: 00000213 ORIG_RAX: 0000000000000001
[ 173.129819] RAX: ffffffffffffffda RBX: 00007d68a8858450 RCX: 00007d6a19d82687
[ 173.129967] RDX: 0000000000000003 RSI: 00007d680881a2a0 RDI: 00000000000000a8
[ 173.130115] RBP: 00007d66c7a7f6e0 R08: ffffffffffffffff R09: 0000000000000000
[ 173.130262] R10: 0000000000020000 R11: 0000000000000213 R12: 0000000000000000
[ 173.130410] R13: 0000000070ec9fb8 R14: 00000000000000a8 R15: 00007d66c7a7f75c
[ 173.130559] </TASK>

Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()

[Borislav Petkov]

On May 12, 2025 10:31:23 AM GMT+02:00, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>This fixes unchecked MSR access error on platform without fixed-range
>MTRRs when doing ACPI S3 suspend.

Is this happening on hw which is shipping now and users will see it or is this some new platform which is yet to see the light of day in the future?

-- 
Sent from a small device: formatting sucks and brevity is inevitable.

Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()

[Jiaqing Zhao]

On 2025-05-12 16:46, Borislav Petkov wrote:
> On May 12, 2025 10:31:23 AM GMT+02:00, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>> This fixes unchecked MSR access error on platform without fixed-range
>> MTRRs when doing ACPI S3 suspend.
> 
> Is this happening on hw which is shipping now and users will see it or is this some new platform which is yet to see the light of day in the future?
> 
Actually it is happening on virtualized platform. A recent version of
ACRN hypervisor has removed emulated fixed-range MTRRs.

Thanks,
Jiaqing

Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()

[Borislav Petkov]

On Mon, May 12, 2025 at 05:24:21PM +0800, Jiaqing Zhao wrote:
> Actually it is happening on virtualized platform. A recent version of
> ACRN hypervisor has removed emulated fixed-range MTRRs.

Oh ok, nothing urgent then.

Will queue it for the next merge window.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette