diff mbox

[v5,15/15] arm64: hibernate: Prevent resume from a different kernel version

Message ID 1455637767-31561-16-git-send-email-james.morse@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

James Morse Feb. 16, 2016, 3:49 p.m. UTC 
Resuming using a different kernel version is fragile, while there are
sufficient details in the hibernate arch-header to perform the restore,
changes in the boot process can have a long-lasting impact on the system.
In particular, if the EFI stub causes more memory to be allocated, the
amount of memory left for linux is reduced. If we are lucky, this will
cause restore to fail with the message:
> PM: Image mismatch: memory size
If we are unlucky, the system will explode sometime later when an EFI
runtime services call is made.

Prevent resuming with a different kernel version, by making
HIBERNATE_VERSION the current kernel version.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/kernel/hibernate.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Comments

Pavel Machek Feb. 16, 2016, 8:15 p.m. UTC | #1 
On Tue 2016-02-16 15:49:27, James Morse wrote:
> Resuming using a different kernel version is fragile, while there are
> sufficient details in the hibernate arch-header to perform the restore,
> changes in the boot process can have a long-lasting impact on the system.
> In particular, if the EFI stub causes more memory to be allocated, the
> amount of memory left for linux is reduced. If we are lucky, this will
> cause restore to fail with the message:

Well, this does not close the door completely. 4.6-rc0 is going to be
very different from 4.6-rc1. Better solution would be to increase
version every time EFI stub changes, or maybe record ammount of memory
reserved for EFI.

Best regards,
									Pavel
Chen Yu Feb. 17, 2016, 2:20 a.m. UTC | #2 
> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Pavel Machek
> Sent: Wednesday, February 17, 2016 4:16 AM
> To: James Morse
> Cc: linux-arm-kernel@lists.infradead.org; Will Deacon; Sudeep Holla; Geoff
> Levand; Catalin Marinas; Lorenzo Pieralisi; Mark Rutland; AKASHI Takahiro;
> Marc Zyngier; Rafael J . Wysocki; linux-pm@vger.kernel.org
> Subject: Re: [PATCH v5 15/15] arm64: hibernate: Prevent resume from a
> different kernel version
> 
> On Tue 2016-02-16 15:49:27, James Morse wrote:
> > Resuming using a different kernel version is fragile, while there are
> > sufficient details in the hibernate arch-header to perform the
> > restore, changes in the boot process can have a long-lasting impact on the
> system.
> > In particular, if the EFI stub causes more memory to be allocated, the
> > amount of memory left for linux is reduced. If we are lucky, this will
> > cause restore to fail with the message:
> 
> Well, this does not close the door completely. 4.6-rc0 is going to be very
> different from 4.6-rc1. Better solution would be to increase version every
> time EFI stub changes, or maybe record ammount of memory reserved for
> EFI.
This reminds me a similar problem I once encountered on x86 : - )
The efi memory layout should be strictly the same before/after hibernation, right?
James Morse Feb. 18, 2016, noon UTC | #3 
On 17/02/16 02:20, Chen, Yu C wrote:
>> On Tue 2016-02-16 15:49:27, James Morse wrote:
>>> Resuming using a different kernel version is fragile, while there are
>>> sufficient details in the hibernate arch-header to perform the
>>> restore, changes in the boot process can have a long-lasting impact on the
>> system.
>>> In particular, if the EFI stub causes more memory to be allocated, the
>>> amount of memory left for linux is reduced. If we are lucky, this will
>>> cause restore to fail with the message:
>>
>> Well, this does not close the door completely. 4.6-rc0 is going to be very
>> different from 4.6-rc1. Better solution would be to increase version every
>> time EFI stub changes, or maybe record ammount of memory reserved for
>> EFI.

I hadn't even considered rcs. Maybe this should be changed to UTS_VERSION.
(There isn't enough space for the struct new_utsname)


> This reminds me a similar problem I once encountered on x86 : - )
> The efi memory layout should be strictly the same before/after hibernation, right?

The kernel hopes it is the same, as the page-tables it uses for runtime services
calls are restored along with the rest of memory, but there is the risk
that these don't match the EFI memory map any more.

Even if the amount of memory is the same, the layout might be different. (Core
hibernate code already has a counter of the number of physical pages.)

I'm still digging through the efi code (and spec), but it is fairly easy to
cause the memory map to change before entry to linux. This doesn't seem to be a
problem, as linux happily overwrites most of the allocated areas, so it may not
be as fragile as I thought.

I received the above 'memory size' error when rebasing between v4.4 and
v4.5-rc1, (Ard's KASLR patches may have been involved too).


Thanks,

James
Chen Yu Feb. 20, 2016, 7:16 p.m. UTC | #4 
> -----Original Message-----
> From: James Morse [mailto:james.morse@arm.com]
> Sent: Thursday, February 18, 2016 8:00 PM
> To: Chen, Yu C
> Cc: Pavel Machek; linux-arm-kernel@lists.infradead.org; Will Deacon; Sudeep
> Holla; Geoff Levand; Catalin Marinas; Lorenzo Pieralisi; Mark Rutland; AKASHI
> Takahiro; Marc Zyngier; Rafael J . Wysocki; linux-pm@vger.kernel.org
> Subject: Re: [PATCH v5 15/15] arm64: hibernate: Prevent resume from a
> different kernel version
> 
> On 17/02/16 02:20, Chen, Yu C wrote:
> >> On Tue 2016-02-16 15:49:27, James Morse wrote:
> > This reminds me a similar problem I once encountered on x86 : - ) The
> > efi memory layout should be strictly the same before/after hibernation,
> right?
> 
> The kernel hopes it is the same, as the page-tables it uses for runtime
> services calls are restored along with the rest of memory, but there is the risk
> that these don't match the EFI memory map any more.
> 
> Even if the amount of memory is the same, the layout might be different.
> (Core hibernate code already has a counter of the number of physical pages.)
I've encountered this situation before.
> 
> I'm still digging through the efi code (and spec), but it is fairly easy to cause
> the memory map to change before entry to linux. This doesn't seem to be a
> problem, as linux happily overwrites most of the allocated areas, so it may
> not be as fragile as I thought.
If I did some peripherals plug/unplug after suspend to hibernation,
then the next time it boots up there would likely be a slightly different efi memory map.
The efi provides a callback of get_memory_map to Linux for retrieving the initial map,
which depends on the bios/bootloader probing/enumeration IMO. The difference of efi
map might lead to kernel panic during resume sometimes.

thanks,
yu
Pavel Machek Feb. 20, 2016, 7:57 p.m. UTC | #5 
On Sat 2016-02-20 19:16:59, Chen, Yu C wrote:
> 
> > -----Original Message-----
> > From: James Morse [mailto:james.morse@arm.com]
> > Sent: Thursday, February 18, 2016 8:00 PM
> > To: Chen, Yu C
> > Cc: Pavel Machek; linux-arm-kernel@lists.infradead.org; Will Deacon; Sudeep
> > Holla; Geoff Levand; Catalin Marinas; Lorenzo Pieralisi; Mark Rutland; AKASHI
> > Takahiro; Marc Zyngier; Rafael J . Wysocki; linux-pm@vger.kernel.org
> > Subject: Re: [PATCH v5 15/15] arm64: hibernate: Prevent resume from a
> > different kernel version
> > 
> > On 17/02/16 02:20, Chen, Yu C wrote:
> > >> On Tue 2016-02-16 15:49:27, James Morse wrote:
> > > This reminds me a similar problem I once encountered on x86 : - ) The
> > > efi memory layout should be strictly the same before/after hibernation,
> > right?
> > 
> > The kernel hopes it is the same, as the page-tables it uses for runtime
> > services calls are restored along with the rest of memory, but there is the risk
> > that these don't match the EFI memory map any more.
> > 
> > Even if the amount of memory is the same, the layout might be different.
> > (Core hibernate code already has a counter of the number of physical pages.)
> I've encountered this situation before.
> > 
> > I'm still digging through the efi code (and spec), but it is fairly easy to cause
> > the memory map to change before entry to linux. This doesn't seem to be a
> > problem, as linux happily overwrites most of the allocated areas, so it may
> > not be as fragile as I thought.
> If I did some peripherals plug/unplug after suspend to hibernation,
> then the next time it boots up there would likely be a slightly different efi memory map.
> The efi provides a callback of get_memory_map to Linux for retrieving the initial map,
> which depends on the bios/bootloader probing/enumeration IMO. The difference of efi
> map might lead to kernel panic during resume sometimes.

md5hash of the efi map, and complain to bios vendors if it happens?
Because iirc it is not supposed to.

									Pavel
Chen Yu Feb. 21, 2016, 9:04 a.m. UTC | #6 
> -----Original Message-----
> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Sunday, February 21, 2016 3:57 AM
> To: Chen, Yu C
> Cc: James Morse; linux-arm-kernel@lists.infradead.org; Will Deacon; Sudeep
> Holla; Geoff Levand; Catalin Marinas; Lorenzo Pieralisi; Mark Rutland; AKASHI
> Takahiro; Marc Zyngier; Rafael J . Wysocki; linux-pm@vger.kernel.org
> Subject: Re: [PATCH v5 15/15] arm64: hibernate: Prevent resume from a
> different kernel version
> 
> On Sat 2016-02-20 19:16:59, Chen, Yu C wrote:
> >
> > > -----Original Message-----
> > > From: James Morse [mailto:james.morse@arm.com]
> > > Sent: Thursday, February 18, 2016 8:00 PM
> > > To: Chen, Yu C
> > > Cc: Pavel Machek; linux-arm-kernel@lists.infradead.org; Will Deacon;
> > > Sudeep Holla; Geoff Levand; Catalin Marinas; Lorenzo Pieralisi; Mark
> > > Rutland; AKASHI Takahiro; Marc Zyngier; Rafael J . Wysocki;
> > > linux-pm@vger.kernel.org
> > > Subject: Re: [PATCH v5 15/15] arm64: hibernate: Prevent resume from
> > > a different kernel version
> > >
> > > On 17/02/16 02:20, Chen, Yu C wrote:
> > > >> On Tue 2016-02-16 15:49:27, James Morse wrote:
> > > > This reminds me a similar problem I once encountered on x86 : - )
> > > > The efi memory layout should be strictly the same before/after
> > > > hibernation,
> > > right?
> > >
> > > The kernel hopes it is the same, as the page-tables it uses for
> > > runtime services calls are restored along with the rest of memory,
> > > but there is the risk that these don't match the EFI memory map any
> more.
> > >
> > > Even if the amount of memory is the same, the layout might be different.
> > > (Core hibernate code already has a counter of the number of physical
> > > pages.)
> > I've encountered this situation before.
> > >
> > > I'm still digging through the efi code (and spec), but it is fairly
> > > easy to cause the memory map to change before entry to linux. This
> > > doesn't seem to be a problem, as linux happily overwrites most of
> > > the allocated areas, so it may not be as fragile as I thought.
> > If I did some peripherals plug/unplug after suspend to hibernation,
> > then the next time it boots up there would likely be a slightly different efi
> memory map.
> > The efi provides a callback of get_memory_map to Linux for retrieving
> > the initial map, which depends on the bios/bootloader
> > probing/enumeration IMO. The difference of efi map might lead to kernel
> panic during resume sometimes.
> 
> md5hash of the efi map, and complain to bios vendors if it happens?
> Because iirc it is not supposed to.
Yes, it should not happen, at least for ACPI S4,
however after communicating with some people from bios team, it is said
BIOSen  are designed to  dynamically allocate address space for
resources.  Maybe we can register a panic notifier to check the consistence
of efi map md5hash.
diff mbox

Patch

diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index fd2faca358f6..99ebdbb06e85 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -56,14 +56,9 @@  extern int in_suspend;
 
 /*
  * This value is written to the hibernate arch header, and prevents resuming
- * from a hibernate image produced by an incompatible kernel. If you change
- * a value that isn't saved/restored by hibernate, you should change this value.
- *
- * For example, if the mair_el1 values used by the kernel are changed, you
- * should prevent resuming from a kernel with incompatible attributes, as these
- * aren't saved/restored.
+ * from a hibernate image produced by a different kernel version.
  */
-#define HIBERNATE_VERSION	KERNEL_VERSION(4, 6, 0)
+#define HIBERNATE_VERSION	LINUX_VERSION_CODE
 
 /*
  * Start/end of the hibernate exit code, this must be copied to a 'safe'