* [PATCH 0/2] x86: restore MISC_ENABLE MSR in realmode wakeup
@ 2011-07-04 22:35 Kees Cook
2011-07-04 22:35 ` [PATCH 1/2] x86: give names to realmode wakeup flags Kees Cook
2011-07-04 22:35 ` [PATCH 2/2] x86: restore MISC_ENABLE MSR in realmode wakeup Kees Cook
0 siblings, 2 replies; 6+ messages in thread
From: Kees Cook @ 2011-07-04 22:35 UTC (permalink / raw)
To: x86, linux-kernel
Cc: Len Brown, Pavel Machek, Rafael J. Wysocki, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin
This is a different approach to handle the situation discussed in
https://lkml.org/lkml/2011/7/1/404, this time restoring the entire
MISC_ENABLE MSR.
-Kees
--
Kees Cook
Ubuntu Security Team
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] x86: give names to realmode wakeup flags
2011-07-04 22:35 [PATCH 0/2] x86: restore MISC_ENABLE MSR in realmode wakeup Kees Cook
@ 2011-07-04 22:35 ` Kees Cook
2011-07-05 22:39 ` H. Peter Anvin
2011-07-04 22:35 ` [PATCH 2/2] x86: restore MISC_ENABLE MSR in realmode wakeup Kees Cook
1 sibling, 1 reply; 6+ messages in thread
From: Kees Cook @ 2011-07-04 22:35 UTC (permalink / raw)
To: x86, linux-kernel
Cc: Len Brown, Pavel Machek, Rafael J. Wysocki, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin
Instead of using literals, use a common set of names for the
user-controlled realmode wakeup flags.
Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
arch/x86/kernel/acpi/realmode/wakemain.c | 6 +++---
arch/x86/kernel/acpi/realmode/wakeup.h | 4 ++++
arch/x86/kernel/acpi/sleep.c | 6 +++---
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/acpi/realmode/wakemain.c b/arch/x86/kernel/acpi/realmode/wakemain.c
index 883962d..1749cde 100644
--- a/arch/x86/kernel/acpi/realmode/wakemain.c
+++ b/arch/x86/kernel/acpi/realmode/wakemain.c
@@ -67,13 +67,13 @@ void main(void)
if (wakeup_header.real_magic != 0x12345678)
while (1);
- if (wakeup_header.realmode_flags & 4)
+ if (wakeup_header.realmode_flags & WAKE_FLAG_BEEP)
send_morse("...-");
- if (wakeup_header.realmode_flags & 1)
+ if (wakeup_header.realmode_flags & WAKE_FLAG_BIOS)
asm volatile("lcallw $0xc000,$3");
- if (wakeup_header.realmode_flags & 2) {
+ if (wakeup_header.realmode_flags & WAKE_FLAG_MODE) {
/* Need to call BIOS */
probe_cards(0);
set_mode(wakeup_header.video_mode);
diff --git a/arch/x86/kernel/acpi/realmode/wakeup.h b/arch/x86/kernel/acpi/realmode/wakeup.h
index e1828c0..88f0e6c 100644
--- a/arch/x86/kernel/acpi/realmode/wakeup.h
+++ b/arch/x86/kernel/acpi/realmode/wakeup.h
@@ -39,4 +39,8 @@ extern struct wakeup_header wakeup_header;
#define WAKEUP_HEADER_SIGNATURE 0x51ee1111
#define WAKEUP_END_SIGNATURE 0x65a22c82
+#define WAKEUP_FLAG_BIOS 1
+#define WAKEUP_FLAG_MODE 2
+#define WAKEUP_FLAG_BEEP 4
+
#endif /* ARCH_X86_KERNEL_ACPI_RM_WAKEUP_H */
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 18a857b..cb968e5 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -104,11 +104,11 @@ static int __init acpi_sleep_setup(char *str)
{
while ((str != NULL) && (*str != '\0')) {
if (strncmp(str, "s3_bios", 7) == 0)
- acpi_realmode_flags |= 1;
+ acpi_realmode_flags |= WAKEUP_FLAG_BIOS;
if (strncmp(str, "s3_mode", 7) == 0)
- acpi_realmode_flags |= 2;
+ acpi_realmode_flags |= WAKEUP_FLAG_MODE;
if (strncmp(str, "s3_beep", 7) == 0)
- acpi_realmode_flags |= 4;
+ acpi_realmode_flags |= WAKEUP_FLAG_BEEP;
#ifdef CONFIG_HIBERNATION
if (strncmp(str, "s4_nohwsig", 10) == 0)
acpi_no_s4_hw_signature();
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] x86: restore MISC_ENABLE MSR in realmode wakeup
2011-07-04 22:35 [PATCH 0/2] x86: restore MISC_ENABLE MSR in realmode wakeup Kees Cook
2011-07-04 22:35 ` [PATCH 1/2] x86: give names to realmode wakeup flags Kees Cook
@ 2011-07-04 22:35 ` Kees Cook
1 sibling, 0 replies; 6+ messages in thread
From: Kees Cook @ 2011-07-04 22:35 UTC (permalink / raw)
To: x86, linux-kernel
Cc: Len Brown, Pavel Machek, Rafael J. Wysocki, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin
Some BIOSes will reset the Intel MISC_ENABLE MSR (specifically the
XD_DISABLE bit) when resuming from S3, which can interact poorly with
ebba638ae723d8a8fc2f7abce5ec18b688b791d7. In 32bit PAE mode, this can
lead to a fault when EFER is restored by the kernel wakeup routines,
due to it setting the NX bit for a CPU that (thanks to the BIOS reset)
now incorrectly thinks it lacks the NX feature. (64bit is not affected
because it uses a common CPU bring-up that specifically handles the
XD_DISABLE bit.)
The need for MISC_ENABLE being restored so early is specific to the S3
resume path. Normally, MISC_ENABLE is saved in save_processor_state(),
but this happens after the resume header is created, so just reproduce
the logic here. (acpi_suspend_lowlevel() creates the header, calls
do_suspend_lowlevel, which calls save_processor_state(), so the saved
processor context isn't available during resume header creation.)
Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
arch/x86/kernel/acpi/realmode/wakeup.S | 14 ++++++++++++++
arch/x86/kernel/acpi/realmode/wakeup.h | 6 ++++++
arch/x86/kernel/acpi/sleep.c | 6 ++++++
3 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/acpi/realmode/wakeup.S b/arch/x86/kernel/acpi/realmode/wakeup.S
index ead21b6..b4fd836 100644
--- a/arch/x86/kernel/acpi/realmode/wakeup.S
+++ b/arch/x86/kernel/acpi/realmode/wakeup.S
@@ -28,6 +28,8 @@ pmode_cr3: .long 0 /* Saved %cr3 */
pmode_cr4: .long 0 /* Saved %cr4 */
pmode_efer: .quad 0 /* Saved EFER */
pmode_gdt: .quad 0
+pmode_misc_en: .quad 0 /* Saved MISC_ENABLE MSR */
+pmode_behavior: .long 0 /* Wakeup behavior flags */
realmode_flags: .long 0
real_magic: .long 0
trampoline_segment: .word 0
@@ -91,6 +93,18 @@ wakeup_code:
/* Call the C code */
calll main
+ /* Restore MISC_ENABLE before entering protected mode, in case
+ BIOS decided to clear XD_DISABLE during S3. */
+ movl pmode_behavior, %eax
+ btl $WAKEUP_BEHAVIOR_RESTORE_MISC_ENABLE, %eax
+ jnc 1f
+
+ movl pmode_misc_en, %eax
+ movl pmode_misc_en + 4, %edx
+ movl $MSR_IA32_MISC_ENABLE, %ecx
+ wrmsr
+1:
+
/* Do any other stuff... */
#ifndef CONFIG_64BIT
diff --git a/arch/x86/kernel/acpi/realmode/wakeup.h b/arch/x86/kernel/acpi/realmode/wakeup.h
index 88f0e6c..5487db0 100644
--- a/arch/x86/kernel/acpi/realmode/wakeup.h
+++ b/arch/x86/kernel/acpi/realmode/wakeup.h
@@ -21,6 +21,9 @@ struct wakeup_header {
u32 pmode_efer_low; /* Protected mode EFER */
u32 pmode_efer_high;
u64 pmode_gdt;
+ u32 pmode_misc_en_low; /* Protected mode MISC_ENABLE */
+ u32 pmode_misc_en_high;
+ u32 pmode_behavior; /* Wakeup routine behavior flags */
u32 realmode_flags;
u32 real_magic;
u16 trampoline_segment; /* segment with trampoline code, 64-bit only */
@@ -43,4 +46,7 @@ extern struct wakeup_header wakeup_header;
#define WAKEUP_FLAG_MODE 2
#define WAKEUP_FLAG_BEEP 4
+/* Wakeup behavior bits */
+#define WAKEUP_BEHAVIOR_RESTORE_MISC_ENABLE 0
+
#endif /* ARCH_X86_KERNEL_ACPI_RM_WAKEUP_H */
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index cb968e5..12115a9 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -77,6 +77,12 @@ int acpi_suspend_lowlevel(void)
header->pmode_cr0 = read_cr0();
header->pmode_cr4 = read_cr4_safe();
+ header->pmode_behavior = 0;
+ if (!rdmsr_safe(MSR_IA32_MISC_ENABLE,
+ &header->pmode_misc_en_low,
+ &header->pmode_misc_en_high))
+ header->pmode_behavior |=
+ (1 << WAKEUP_BEHAVIOR_RESTORE_MISC_ENABLE);
header->realmode_flags = acpi_realmode_flags;
header->real_magic = 0x12345678;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86: give names to realmode wakeup flags
2011-07-04 22:35 ` [PATCH 1/2] x86: give names to realmode wakeup flags Kees Cook
@ 2011-07-05 22:39 ` H. Peter Anvin
2011-07-06 1:27 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: H. Peter Anvin @ 2011-07-05 22:39 UTC (permalink / raw)
To: Kees Cook
Cc: x86, linux-kernel, Len Brown, Pavel Machek, Rafael J. Wysocki,
Thomas Gleixner, Ingo Molnar
On 07/04/2011 03:35 PM, Kees Cook wrote:
> Instead of using literals, use a common set of names for the
> user-controlled realmode wakeup flags.
>
> Signed-off-by: Kees Cook <kees.cook@canonical.com>
I'm sorry, but I really have to complain about this:
This was a very unfriendly thing to do. You took a patch that is a bug
fix to be considered for -stable, and you applied it *on top of a
cleanup patch*. They should not have been part of the same patchset,
but *certainly* not in that order.
Please resubmit.
-hpa
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86: give names to realmode wakeup flags
2011-07-05 22:39 ` H. Peter Anvin
@ 2011-07-06 1:27 ` Kees Cook
2011-07-06 2:41 ` H. Peter Anvin
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2011-07-06 1:27 UTC (permalink / raw)
To: H. Peter Anvin
Cc: x86, linux-kernel, Len Brown, Pavel Machek, Rafael J. Wysocki,
Thomas Gleixner, Ingo Molnar
On Tue, Jul 05, 2011 at 03:39:54PM -0700, H. Peter Anvin wrote:
> On 07/04/2011 03:35 PM, Kees Cook wrote:
> > Instead of using literals, use a common set of names for the
> > user-controlled realmode wakeup flags.
> >
> > Signed-off-by: Kees Cook <kees.cook@canonical.com>
>
> I'm sorry, but I really have to complain about this:
>
> This was a very unfriendly thing to do.
Obviously I wasn't trying to be unfriendly. :P
> You took a patch that is a bug fix to be considered for -stable,
> and you applied it *on top of a cleanup patch*.
It wasn't clear to me if the MISC_ENABLE reload should be considered for
stable (it does technically "more" than my original patch, and changes
the resume header structure, etc). If it should be forwarded to -stable,
that's fine too. I just didn't want to presume.
> They should not have been part of the same patchset,
> but *certainly* not in that order.
Since they hit the same .h file in the same location, I wasn't sure what
order to do it in. It seemed unhelpful to send them separately.
> Please resubmit.
Sure thing -- in the opposite order, or totally separate from each other?
-Kees
--
Kees Cook
Ubuntu Security Team
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86: give names to realmode wakeup flags
2011-07-06 1:27 ` Kees Cook
@ 2011-07-06 2:41 ` H. Peter Anvin
0 siblings, 0 replies; 6+ messages in thread
From: H. Peter Anvin @ 2011-07-06 2:41 UTC (permalink / raw)
To: Kees Cook
Cc: x86, linux-kernel, Len Brown, Pavel Machek, Rafael J. Wysocki,
Thomas Gleixner, Ingo Molnar
On 07/05/2011 06:27 PM, Kees Cook wrote:
>
>> Please resubmit.
>
> Sure thing -- in the opposite order, or totally separate from each other?
>
Either is fine.
-hpa
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-07-06 2:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-04 22:35 [PATCH 0/2] x86: restore MISC_ENABLE MSR in realmode wakeup Kees Cook
2011-07-04 22:35 ` [PATCH 1/2] x86: give names to realmode wakeup flags Kees Cook
2011-07-05 22:39 ` H. Peter Anvin
2011-07-06 1:27 ` Kees Cook
2011-07-06 2:41 ` H. Peter Anvin
2011-07-04 22:35 ` [PATCH 2/2] x86: restore MISC_ENABLE MSR in realmode wakeup Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.