| 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
| ||
|
Date: Mon, 18 Aug 2008 12:16:39 +0200
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Ingo Molnar <mingo@...e.hu>
Cc: David Fries <david@...es.net>, linux-kernel@...r.kernel.org,
Pavel Machek <pavel@....cz>, "H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] Fix i486 suspend to disk CR4 oops
On Monday, 18 of August 2008, Ingo Molnar wrote:
>
> * David Fries <david@...es.net> wrote:
>
> > arch/x86/power/cpu_32.c __save_processor_state calls read_cr4() only a
> > i486 CPU doesn't have the CR4 register. Trying to read it produces an
> > invalid opcode oops during suspend to disk.
> >
> > Added the check (boot_cpu_data.x86 >= 5) before reading the register.
> > If the value to be written is zero the write is skipped.
> >
> > arch/x86/power/hibernate_asm_32.S
> > done: swapped the use of %eax and %ecx to use jecxz for
> > the zero test and jump over store to %cr4.
> > restore_image: s/%ecx/%eax/ to be consistent with done:
> >
> > In addition to __save_processor_state, acpi_save_state_mem,
> > efi_call_phys_prelog, and efi_call_phys_epilog had checks added (acpi
> > restore was in assembly and already had a check for non-zero). There
> > were other reads and writes of CR4, but MCE and virtualization
> > shouldn't be executed on a i486 anyway.
> >
> > Signed-off-by: David Fries <david@...es.net>
>
> applied to tip/x86/urgent, thanks David. I've changed the conditions to
> read_cr4_safe() instead - that's cleaner. Could you please check whether
> the patch below works fine too on your box?
>
> Rafael, Pavel - does the commit below look good to you too?
>
> Ingo
>
> ---------------------->
> From e437fa5586f2e3b2aaeba649fae52be1f9a6eabb Mon Sep 17 00:00:00 2001
> From: David Fries <david@...es.net>
> Date: Sun, 17 Aug 2008 23:03:40 -0500
> Subject: [PATCH] x86: fix i486 suspend to disk CR4 oops
>
> arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
> only a i486 CPU doesn't have the CR4 register. Trying to read it
> produces an invalid opcode oops during suspend to disk.
>
> Use the safe rc4 reading op instead. If the value to be written is
> zero the write is skipped.
>
> arch/x86/power/hibernate_asm_32.S
> done: swapped the use of %eax and %ecx to use jecxz for
> the zero test and jump over store to %cr4.
> restore_image: s/%ecx/%eax/ to be consistent with done:
>
> In addition to __save_processor_state, acpi_save_state_mem,
> efi_call_phys_prelog, and efi_call_phys_epilog had checks added
> (acpi restore was in assembly and already had a check for
> non-zero). There were other reads and writes of CR4, but MCE and
> virtualization shouldn't be executed on a i486 anyway.
>
> Signed-off-by: David Fries <david@...es.net>
> Signed-off-by: Ingo Molnar <mingo@...e.hu>
Acked-by: Rafael J. Wysocki <rjw@...k.pl>
> ---
> arch/x86/kernel/acpi/sleep.c | 2 +-
> arch/x86/kernel/efi_32.c | 4 ++--
> arch/x86/power/cpu_32.c | 6 ++++--
> arch/x86/power/hibernate_asm_32.S | 26 +++++++++++++++-----------
> 4 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
> index 81e5ab6..426e5d9 100644
> --- a/arch/x86/kernel/acpi/sleep.c
> +++ b/arch/x86/kernel/acpi/sleep.c
> @@ -86,7 +86,7 @@ int acpi_save_state_mem(void)
> #endif /* !CONFIG_64BIT */
>
> header->pmode_cr0 = read_cr0();
> - header->pmode_cr4 = read_cr4();
> + header->pmode_cr4 = read_cr4_safe();
> header->realmode_flags = acpi_realmode_flags;
> header->real_magic = 0x12345678;
>
> diff --git a/arch/x86/kernel/efi_32.c b/arch/x86/kernel/efi_32.c
> index 4b63c8e..5cab48e 100644
> --- a/arch/x86/kernel/efi_32.c
> +++ b/arch/x86/kernel/efi_32.c
> @@ -53,7 +53,7 @@ void efi_call_phys_prelog(void)
> * directory. If I have PAE, I just need to duplicate one entry in
> * page directory.
> */
> - cr4 = read_cr4();
> + cr4 = read_cr4_safe();
>
> if (cr4 & X86_CR4_PAE) {
> efi_bak_pg_dir_pointer[0].pgd =
> @@ -91,7 +91,7 @@ void efi_call_phys_epilog(void)
> gdt_descr.size = GDT_SIZE - 1;
> load_gdt(&gdt_descr);
>
> - cr4 = read_cr4();
> + cr4 = read_cr4_safe();
>
> if (cr4 & X86_CR4_PAE) {
> swapper_pg_dir[pgd_index(0)].pgd =
> diff --git a/arch/x86/power/cpu_32.c b/arch/x86/power/cpu_32.c
> index 7dc5d5c..d3e083d 100644
> --- a/arch/x86/power/cpu_32.c
> +++ b/arch/x86/power/cpu_32.c
> @@ -45,7 +45,7 @@ static void __save_processor_state(struct saved_context *ctxt)
> ctxt->cr0 = read_cr0();
> ctxt->cr2 = read_cr2();
> ctxt->cr3 = read_cr3();
> - ctxt->cr4 = read_cr4();
> + ctxt->cr4 = read_cr4_safe();
> }
>
> /* Needed by apm.c */
> @@ -98,7 +98,9 @@ static void __restore_processor_state(struct saved_context *ctxt)
> /*
> * control registers
> */
> - write_cr4(ctxt->cr4);
> + /* cr4 was introduced in the Pentium CPU */
> + if (ctxt->cr4)
> + write_cr4(ctxt->cr4);
> write_cr3(ctxt->cr3);
> write_cr2(ctxt->cr2);
> write_cr0(ctxt->cr0);
> diff --git a/arch/x86/power/hibernate_asm_32.S b/arch/x86/power/hibernate_asm_32.S
> index b95aa6c..4fc7e87 100644
> --- a/arch/x86/power/hibernate_asm_32.S
> +++ b/arch/x86/power/hibernate_asm_32.S
> @@ -28,9 +28,9 @@ ENTRY(swsusp_arch_suspend)
> ret
>
> ENTRY(restore_image)
> - movl resume_pg_dir, %ecx
> - subl $__PAGE_OFFSET, %ecx
> - movl %ecx, %cr3
> + movl resume_pg_dir, %eax
> + subl $__PAGE_OFFSET, %eax
> + movl %eax, %cr3
>
> movl restore_pblist, %edx
> .p2align 4,,7
> @@ -52,17 +52,21 @@ copy_loop:
>
> done:
> /* go back to the original page tables */
> - movl $swapper_pg_dir, %ecx
> - subl $__PAGE_OFFSET, %ecx
> - movl %ecx, %cr3
> + movl $swapper_pg_dir, %eax
> + subl $__PAGE_OFFSET, %eax
> + movl %eax, %cr3
> /* Flush TLB, including "global" things (vmalloc) */
> - movl mmu_cr4_features, %eax
> - movl %eax, %edx
> + movl mmu_cr4_features, %ecx
> + jecxz 1f # cr4 Pentium and higher, skip if zero
> + movl %ecx, %edx
> andl $~(1<<7), %edx; # PGE
> movl %edx, %cr4; # turn off PGE
> - movl %cr3, %ecx; # flush TLB
> - movl %ecx, %cr3
> - movl %eax, %cr4; # turn PGE back on
> +1:
> + movl %cr3, %eax; # flush TLB
> + movl %eax, %cr3
> + jecxz 1f # cr4 Pentium and higher, skip if zero
> + movl %ecx, %cr4; # turn PGE back on
> +1:
>
> movl saved_context_esp, %esp
> movl saved_context_ebp, %ebp
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists