Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1457786 > unrolled thread

[Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

Back to article view | Back to linux.kernel

Contents

  [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-08-08 15:30 +0200
    Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Borislav Petkov <bp@suse.de> - 2016-08-08 15:50 +0200
      Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-08 16:00 +0200
        Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Thomas Garnier <thgarnie@google.com> - 2016-08-08 20:10 +0200
          Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-08 22:10 +0200
        Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-09 13:50 +0200
    Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Jiri Kosina <jikos@kernel.org> - 2016-08-09 11:30 +0200
      Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-09 14:00 +0200
        Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Jiri Kosina <jikos@kernel.org> - 2016-08-09 15:00 +0200
          Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Jiri Kosina <jikos@kernel.org> - 2016-08-09 15:40 +0200
            Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Jiri Kosina <jikos@kernel.org> - 2016-08-09 17:10 +0200
              Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-09 18:20 +0200
                Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Thomas Garnier <thgarnie@google.com> - 2016-08-09 18:30 +0200
                  Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-09 23:00 +0200
                Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Jiri Kosina <jikos@kernel.org> - 2016-08-09 22:10 +0200
                  Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-09 23:30 +0200
                    Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-08-10 02:20 +0200
                      Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Thomas Garnier <thgarnie@google.com> - 2016-08-10 20:40 +0200
                      Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Jiri Kosina <jikos@kernel.org> - 2016-08-10 21:00 +0200
                        Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Borislav Petkov <bp@suse.de> - 2016-08-10 21:00 +0200
                          Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-10 23:00 +0200
                            Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly "Rafael J. Wysocki" <rafael@kernel.org> - 2016-08-10 23:20 +0200
                        Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Jiri Kosina <jikos@kernel.org> - 2016-08-10 21:00 +0200
                          Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Thomas Garnier <thgarnie@google.com> - 2016-08-10 22:00 +0200
                            Re: [Resend][PATCH] x86/power/64: Always create temporary identity  mapping correctly Jiri Kosina <jikos@kernel.org> - 2016-08-10 22:50 +0200
                        Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-08-10 22:10 +0200

Page 1 of 2  [1] 2  Next page →

#1457786 — [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The low-level resume-from-hibernation code on x86-64 uses
kernel_ident_mapping_init() to create the temoprary identity mapping,
but that function assumes that the offset between kernel virtual
addresses and physical addresses is aligned on the PGD level.

However, with a randomized identity mapping base, it may be aligned
on the PUD level and if that happens, the temporary identity mapping
created by set_up_temporary_mappings() will not reflect the actual
kernel identity mapping and the image restoration will fail as a
result (leading to a kernel panic most of the time).

To fix this problem, rework kernel_ident_mapping_init() to support
unaligned offsets between KVA and PA up to the PMD level and make
set_up_temporary_mappings() use it as approprtiate.

Reported-by: Thomas Garnier <thgarnie@google.com>
Suggested-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Yinghai Lu <yinghai@kernel.org>
---

This is sort of urgent, because hibernation doesn't work with KASLR on x86-64
in 4.8-rc1 AFAICS and this should make them work together again.

Unless anyone sees any problems with it, I'll queue it up for 4.8-rc2.

Thomas, would it be possible to test it with KASLR enabled, please?

Thanks,
Rafael

---
 arch/x86/include/asm/init.h   |    4 ++--
 arch/x86/mm/ident_map.c       |   19 +++++++++++--------
 arch/x86/power/hibernate_64.c |    2 +-
 3 files changed, 14 insertions(+), 11 deletions(-)

Index: linux-pm/arch/x86/include/asm/init.h
===================================================================
--- linux-pm.orig/arch/x86/include/asm/init.h
+++ linux-pm/arch/x86/include/asm/init.h
@@ -5,10 +5,10 @@ struct x86_mapping_info {
 	void *(*alloc_pgt_page)(void *); /* allocate buf for page table */
 	void *context;			 /* context for alloc_pgt_page */
 	unsigned long pmd_flag;		 /* page flag for PMD entry */
-	bool kernel_mapping;		 /* kernel mapping or ident mapping */
+	unsigned long offset;		 /* ident mapping offset */
 };
 
 int kernel_ident_mapping_init(struct x86_mapping_info *info, pgd_t *pgd_page,
-				unsigned long addr, unsigned long end);
+				unsigned long pstart, unsigned long pend);
 
 #endif /* _ASM_X86_INIT_H */
Index: linux-pm/arch/x86/mm/ident_map.c
===================================================================
--- linux-pm.orig/arch/x86/mm/ident_map.c
+++ linux-pm/arch/x86/mm/ident_map.c
@@ -3,15 +3,17 @@
  * included by both the compressed kernel and the regular kernel.
  */
 
-static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
+static void ident_pmd_init(struct x86_mapping_info *info, pmd_t *pmd_page,
 			   unsigned long addr, unsigned long end)
 {
 	addr &= PMD_MASK;
 	for (; addr < end; addr += PMD_SIZE) {
 		pmd_t *pmd = pmd_page + pmd_index(addr);
 
-		if (!pmd_present(*pmd))
-			set_pmd(pmd, __pmd(addr | pmd_flag));
+		if (pmd_present(*pmd))
+			continue;
+
+		set_pmd(pmd, __pmd((addr - info->offset) | info->pmd_flag));
 	}
 }
 
@@ -30,13 +32,13 @@ static int ident_pud_init(struct x86_map
 
 		if (pud_present(*pud)) {
 			pmd = pmd_offset(pud, 0);
-			ident_pmd_init(info->pmd_flag, pmd, addr, next);
+			ident_pmd_init(info, pmd, addr, next);
 			continue;
 		}
 		pmd = (pmd_t *)info->alloc_pgt_page(info->context);
 		if (!pmd)
 			return -ENOMEM;
-		ident_pmd_init(info->pmd_flag, pmd, addr, next);
+		ident_pmd_init(info, pmd, addr, next);
 		set_pud(pud, __pud(__pa(pmd) | _KERNPG_TABLE));
 	}
 
@@ -44,14 +46,15 @@ static int ident_pud_init(struct x86_map
 }
 
 int kernel_ident_mapping_init(struct x86_mapping_info *info, pgd_t *pgd_page,
-			      unsigned long addr, unsigned long end)
+			      unsigned long pstart, unsigned long pend)
 {
+	unsigned long addr = pstart + info->offset;
+	unsigned long end = pend + info->offset;
 	unsigned long next;
 	int result;
-	int off = info->kernel_mapping ? pgd_index(__PAGE_OFFSET) : 0;
 
 	for (; addr < end; addr = next) {
-		pgd_t *pgd = pgd_page + pgd_index(addr) + off;
+		pgd_t *pgd = pgd_page + pgd_index(addr);
 		pud_t *pud;
 
 		next = (addr & PGDIR_MASK) + PGDIR_SIZE;
Index: linux-pm/arch/x86/power/hibernate_64.c
===================================================================
--- linux-pm.orig/arch/x86/power/hibernate_64.c
+++ linux-pm/arch/x86/power/hibernate_64.c
@@ -87,7 +87,7 @@ static int set_up_temporary_mappings(voi
 	struct x86_mapping_info info = {
 		.alloc_pgt_page	= alloc_pgt_page,
 		.pmd_flag	= __PAGE_KERNEL_LARGE_EXEC,
-		.kernel_mapping = true,
+		.offset		= __PAGE_OFFSET,
 	};
 	unsigned long mstart, mend;
 	pgd_t *pgd;

[toc] | [next] | [standalone]

#1457802 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The low-level resume-from-hibernation code on x86-64 uses
> kernel_ident_mapping_init() to create the temoprary identity mapping,
> but that function assumes that the offset between kernel virtual
> addresses and physical addresses is aligned on the PGD level.
> 
> However, with a randomized identity mapping base, it may be aligned
> on the PUD level and if that happens, the temporary identity mapping
> created by set_up_temporary_mappings() will not reflect the actual
> kernel identity mapping and the image restoration will fail as a
> result (leading to a kernel panic most of the time).
> 
> To fix this problem, rework kernel_ident_mapping_init() to support
> unaligned offsets between KVA and PA up to the PMD level and make
> set_up_temporary_mappings() use it as approprtiate.
> 
> Reported-by: Thomas Garnier <thgarnie@google.com>

Reported-by: Borislav Petkov <bp@suse.de>

> Suggested-by: Yinghai Lu <yinghai@kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Yinghai Lu <yinghai@kernel.org>
> ---
> 
> This is sort of urgent, because hibernation doesn't work with KASLR on x86-64
> in 4.8-rc1 AFAICS and this should make them work together again.
> 
> Unless anyone sees any problems with it, I'll queue it up for 4.8-rc2.
> 
> Thomas, would it be possible to test it with KASLR enabled, please?

Is that the only patch which needs to be tested? Ontop of which tree?

CONFIG_RANDOMIZE_MEMORY blew up s2d on my laptop here so I'll run it
once I have the required info from you :)

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--

[toc] | [prev] | [next] | [standalone]

#1457812 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov <bp@suse.de> wrote:
> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> The low-level resume-from-hibernation code on x86-64 uses
>> kernel_ident_mapping_init() to create the temoprary identity mapping,
>> but that function assumes that the offset between kernel virtual
>> addresses and physical addresses is aligned on the PGD level.
>>
>> However, with a randomized identity mapping base, it may be aligned
>> on the PUD level and if that happens, the temporary identity mapping
>> created by set_up_temporary_mappings() will not reflect the actual
>> kernel identity mapping and the image restoration will fail as a
>> result (leading to a kernel panic most of the time).
>>
>> To fix this problem, rework kernel_ident_mapping_init() to support
>> unaligned offsets between KVA and PA up to the PMD level and make
>> set_up_temporary_mappings() use it as approprtiate.
>>
>> Reported-by: Thomas Garnier <thgarnie@google.com>
>
> Reported-by: Borislav Petkov <bp@suse.de>
>
>> Suggested-by: Yinghai Lu <yinghai@kernel.org>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Acked-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>
>> This is sort of urgent, because hibernation doesn't work with KASLR on x86-64
>> in 4.8-rc1 AFAICS and this should make them work together again.
>>
>> Unless anyone sees any problems with it, I'll queue it up for 4.8-rc2.
>>
>> Thomas, would it be possible to test it with KASLR enabled, please?
>
> Is that the only patch which needs to be tested? Ontop of which tree?

That should be the only one on top of plain 4.8-rc1.

If it doesn't help, we need more work to do. :-)

> CONFIG_RANDOMIZE_MEMORY blew up s2d on my laptop here so I'll run it
> once I have the required info from you :)

Thanks!

Best,
Rafael

[toc] | [prev] | [next] | [standalone]

#1457964 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Mon, Aug 8, 2016 at 6:54 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov <bp@suse.de> wrote:
>> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> The low-level resume-from-hibernation code on x86-64 uses
>>> kernel_ident_mapping_init() to create the temoprary identity mapping,
>>> but that function assumes that the offset between kernel virtual
>>> addresses and physical addresses is aligned on the PGD level.
>>>
>>> However, with a randomized identity mapping base, it may be aligned
>>> on the PUD level and if that happens, the temporary identity mapping
>>> created by set_up_temporary_mappings() will not reflect the actual
>>> kernel identity mapping and the image restoration will fail as a
>>> result (leading to a kernel panic most of the time).
>>>
>>> To fix this problem, rework kernel_ident_mapping_init() to support
>>> unaligned offsets between KVA and PA up to the PMD level and make
>>> set_up_temporary_mappings() use it as approprtiate.
>>>
>>> Reported-by: Thomas Garnier <thgarnie@google.com>
>>
>> Reported-by: Borislav Petkov <bp@suse.de>
>>
>>> Suggested-by: Yinghai Lu <yinghai@kernel.org>
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> Acked-by: Yinghai Lu <yinghai@kernel.org>
>>> ---
>>>
>>> This is sort of urgent, because hibernation doesn't work with KASLR on x86-64
>>> in 4.8-rc1 AFAICS and this should make them work together again.
>>>
>>> Unless anyone sees any problems with it, I'll queue it up for 4.8-rc2.
>>>
>>> Thomas, would it be possible to test it with KASLR enabled, please?
>>

I tested it on my setup couple times. Worked well.

>> Is that the only patch which needs to be tested? Ontop of which tree?
>
> That should be the only one on top of plain 4.8-rc1.
>
> If it doesn't help, we need more work to do. :-)
>
>> CONFIG_RANDOMIZE_MEMORY blew up s2d on my laptop here so I'll run it
>> once I have the required info from you :)
>
> Thanks!
>
> Best,
> Rafael

[toc] | [prev] | [next] | [standalone]

#1458206 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Mon, Aug 8, 2016 at 8:00 PM, Thomas Garnier <thgarnie@google.com> wrote:
> On Mon, Aug 8, 2016 at 6:54 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov <bp@suse.de> wrote:
>>> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>
>>>> The low-level resume-from-hibernation code on x86-64 uses
>>>> kernel_ident_mapping_init() to create the temoprary identity mapping,
>>>> but that function assumes that the offset between kernel virtual
>>>> addresses and physical addresses is aligned on the PGD level.
>>>>
>>>> However, with a randomized identity mapping base, it may be aligned
>>>> on the PUD level and if that happens, the temporary identity mapping
>>>> created by set_up_temporary_mappings() will not reflect the actual
>>>> kernel identity mapping and the image restoration will fail as a
>>>> result (leading to a kernel panic most of the time).
>>>>
>>>> To fix this problem, rework kernel_ident_mapping_init() to support
>>>> unaligned offsets between KVA and PA up to the PMD level and make
>>>> set_up_temporary_mappings() use it as approprtiate.
>>>>
>>>> Reported-by: Thomas Garnier <thgarnie@google.com>
>>>
>>> Reported-by: Borislav Petkov <bp@suse.de>
>>>
>>>> Suggested-by: Yinghai Lu <yinghai@kernel.org>
>>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>> Acked-by: Yinghai Lu <yinghai@kernel.org>
>>>> ---
>>>>
>>>> This is sort of urgent, because hibernation doesn't work with KASLR on x86-64
>>>> in 4.8-rc1 AFAICS and this should make them work together again.
>>>>
>>>> Unless anyone sees any problems with it, I'll queue it up for 4.8-rc2.
>>>>
>>>> Thomas, would it be possible to test it with KASLR enabled, please?
>>>
>
> I tested it on my setup couple times. Worked well.

Thanks!

[toc] | [prev] | [next] | [standalone]

#1458639 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, Aug 9, 2016 at 9:02 AM, Borislav Petkov <bp@suse.de> wrote:
> On Mon, Aug 08, 2016 at 03:54:48PM +0200, Rafael J. Wysocki wrote:
>> That should be the only one on top of plain 4.8-rc1.
>>
>> If it doesn't help, we need more work to do. :-)
>
> Yes, we do.
>
> The machine triple-faults *after* reading up the hibernation image.
> It hits 100%, then tries to switch to the boot kernel and BOOM, BIOS
> screen.
>
> I'm attaching the .config in case you want to reproduce it. The machine
> is an IVB thinkpad x230.

Yes, I'm going to try to reproduce it.

I'm wondering what the difference between your .config and the Thomas'
.config is, as he has CONFIG_RANDOMIZE_MEMORY=y set too.

Thomas, can you attach your .config, please?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1458539 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Mon, 8 Aug 2016, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The low-level resume-from-hibernation code on x86-64 uses
> kernel_ident_mapping_init() to create the temoprary identity mapping,
> but that function assumes that the offset between kernel virtual
> addresses and physical addresses is aligned on the PGD level.
> 
> However, with a randomized identity mapping base, it may be aligned
> on the PUD level and if that happens, the temporary identity mapping
> created by set_up_temporary_mappings() will not reflect the actual
> kernel identity mapping and the image restoration will fail as a
> result (leading to a kernel panic most of the time).
> 
> To fix this problem, rework kernel_ident_mapping_init() to support
> unaligned offsets between KVA and PA up to the PMD level and make
> set_up_temporary_mappings() use it as approprtiate.
> 
> Reported-by: Thomas Garnier <thgarnie@google.com>
> Suggested-by: Yinghai Lu <yinghai@kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Yinghai Lu <yinghai@kernel.org>
> ---
> 
> This is sort of urgent, because hibernation doesn't work with KASLR on x86-64
> in 4.8-rc1 AFAICS and this should make them work together again.
> 
> Unless anyone sees any problems with it, I'll queue it up for 4.8-rc2.
> 
> Thomas, would it be possible to test it with KASLR enabled, please?

Unfortunately this applied on top of -rc1 still doesn't solve the reboot 
after reading hibernation image (I'd guess due to triple fault) with 
CONFIG_RANDOMIZE_MEMORY=y on my system.

With CONFIG_RANDOMIZE_MEMORY=n, the system resumes correctly.

-- 
Jiri Kosina
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1458648 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, Aug 9, 2016 at 11:23 AM, Jiri Kosina <jikos@kernel.org> wrote:
> On Mon, 8 Aug 2016, Rafael J. Wysocki wrote:
>
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> The low-level resume-from-hibernation code on x86-64 uses
>> kernel_ident_mapping_init() to create the temoprary identity mapping,
>> but that function assumes that the offset between kernel virtual
>> addresses and physical addresses is aligned on the PGD level.
>>
>> However, with a randomized identity mapping base, it may be aligned
>> on the PUD level and if that happens, the temporary identity mapping
>> created by set_up_temporary_mappings() will not reflect the actual
>> kernel identity mapping and the image restoration will fail as a
>> result (leading to a kernel panic most of the time).
>>
>> To fix this problem, rework kernel_ident_mapping_init() to support
>> unaligned offsets between KVA and PA up to the PMD level and make
>> set_up_temporary_mappings() use it as approprtiate.
>>
>> Reported-by: Thomas Garnier <thgarnie@google.com>
>> Suggested-by: Yinghai Lu <yinghai@kernel.org>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Acked-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>
>> This is sort of urgent, because hibernation doesn't work with KASLR on x86-64
>> in 4.8-rc1 AFAICS and this should make them work together again.
>>
>> Unless anyone sees any problems with it, I'll queue it up for 4.8-rc2.
>>
>> Thomas, would it be possible to test it with KASLR enabled, please?
>
> Unfortunately this applied on top of -rc1 still doesn't solve the reboot
> after reading hibernation image (I'd guess due to triple fault) with
> CONFIG_RANDOMIZE_MEMORY=y on my system.
>
> With CONFIG_RANDOMIZE_MEMORY=n, the system resumes correctly.

Here's a list of commits from Thomas that are related to memory randomization.

210e7a43fa90 mm: SLUB freelist randomization
7c00fce98c3e mm: reorganize SLAB freelist randomization
4ff5308744f5 x86/mm: Do not reference phys addr beyond kernel
90397a417796 x86/mm: Add memory hotplug support for KASLR memory randomization
a95ae27c2ee1 x86/mm: Enable KASLR for vmalloc memory regions
021182e52fe0 x86/mm: Enable KASLR for physical mapping memory regions
0483e1fa6e09 x86/mm: Implement ASLR for kernel memory regions
b234e8a09003 x86/mm: Separate variable for trampoline PGD
faa379332f3c x86/mm: Add PUD VA support for physical mapping
59b3d0206d74 x86/mm: Update physical mapping variable names
d899a7d146a2 x86/mm: Refactor KASLR entropy functions

I wonder if it is viable to revert them one by one top-to-bottom and
see which one of them causes things to fail?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1458720 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, 9 Aug 2016, Rafael J. Wysocki wrote:

> Here's a list of commits from Thomas that are related to memory randomization.
> 
> 210e7a43fa90 mm: SLUB freelist randomization
> 7c00fce98c3e mm: reorganize SLAB freelist randomization
> 4ff5308744f5 x86/mm: Do not reference phys addr beyond kernel
> 90397a417796 x86/mm: Add memory hotplug support for KASLR memory randomization
> a95ae27c2ee1 x86/mm: Enable KASLR for vmalloc memory regions
> 021182e52fe0 x86/mm: Enable KASLR for physical mapping memory regions

Okay, I did one-by-one reverts, and the one above, i.e.

	commit 021182e52fe01c1f7b126f97fd6ba048dc4234fd
	Author: Thomas Garnier <thgarnie@google.com>
	Date:   Tue Jun 21 17:47:03 2016 -0700

	    x86/mm: Enable KASLR for physical mapping memory regions

is the one that is the culprit on my machine. With this one reverted, 
resume hibernation doesn't reboot (tripple fault?), but proceeds 
succesfully.

-- 
Jiri Kosina
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1458741 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, 9 Aug 2016, Jiri Kosina wrote:

> > 210e7a43fa90 mm: SLUB freelist randomization
> > 7c00fce98c3e mm: reorganize SLAB freelist randomization
> > 4ff5308744f5 x86/mm: Do not reference phys addr beyond kernel
> > 90397a417796 x86/mm: Add memory hotplug support for KASLR memory randomization
> > a95ae27c2ee1 x86/mm: Enable KASLR for vmalloc memory regions
> > 021182e52fe0 x86/mm: Enable KASLR for physical mapping memory regions
> 
> Okay, I did one-by-one reverts, and the one above, i.e.
> 
> 	commit 021182e52fe01c1f7b126f97fd6ba048dc4234fd
> 	Author: Thomas Garnier <thgarnie@google.com>
> 	Date:   Tue Jun 21 17:47:03 2016 -0700
> 
> 	    x86/mm: Enable KASLR for physical mapping memory regions
> 
> is the one that is the culprit on my machine. With this one reverted, 
> resume hibernation doesn't reboot (tripple fault?), but proceeds 
> succesfully.

As discussed with Rafael privately, I also tried this very patch 
(x86/power/64: Always create temporary identity mapping correctly) on top 
of the reverted revert of 021182e52fe01c1f7b1 (see the full log below), 
but such kernel triple faults on resume as well.

87c38d2 x86/power/64: Always create temporary identity mapping correctly
3cb504a Revert "Revert "x86/mm: Enable KASLR for physical mapping memory regions""
758850d Revert "x86/mm: Enable KASLR for physical mapping memory regions"
4a02dfb Revert "x86/mm: Enable KASLR for vmalloc memory regions"
037863f Revert "x86/mm: Add memory hotplug support for KASLR memory randomization"
3416a21 Revert "x86/mm: Do not reference phys addr beyond kernel"
69227be Revert "mm: reorganize SLAB freelist randomization"
a1d8d71 Revert "mm: SLUB freelist randomization"

IOW, 021182e52f introduces a bug for which there is no existing fix yet.

-- 
Jiri Kosina
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1458834 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, 9 Aug 2016, Thomas Garnier wrote:

> >> Okay, I did one-by-one reverts, and the one above, i.e.
> >>
> >>       commit 021182e52fe01c1f7b126f97fd6ba048dc4234fd
> >>       Author: Thomas Garnier <thgarnie@google.com>
> >>       Date:   Tue Jun 21 17:47:03 2016 -0700
> >>
> >>           x86/mm: Enable KASLR for physical mapping memory regions
> >>
> >> is the one that is the culprit on my machine. With this one reverted,
> >> resume hibernation doesn't reboot (tripple fault?), but proceeds
> >> succesfully.
> 
> My .config is attached. It is basically defconfig (x86_64) + kvmconfig
> plus the following:
> 
> CONFIG_PHYSICAL_START=0x1000000
> CONFIG_RELOCATABLE=y
> CONFIG_RANDOMIZE_BASE=y
> CONFIG_X86_NEED_RELOCS=y
> CONFIG_PHYSICAL_ALIGN=0x1000000
> CONFIG_RANDOMIZE_MEMORY=y
> CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING=0xa
> CONFIG_X86_PTDUMP_CORE=y
> CONFIG_X86_PTDUMP=y
> CONFIG_KALLSYMS=y
> CONFIG_KALLSYMS_ALL=y
> CONFIG_KALLSYMS_ABSOLUTE_PERCPU=y
> CONFIG_KALLSYMS_BASE_RELATIVE=y
> CONFIG_PANIC_ON_OOPS=y
> CONFIG_KGDB=y
> CONFIG_EARLY_PRINTK=y
> CONFIG_EARLY_PRINTK_DBGP=y
> CONFIG_DEBUG_INFO=y
> CONFIG_DEBUG_INFO_DWARF4=y

The config I am reproducing the bug with (on thinkpad x200s) can be found 
at

	http://www.jikos.cz/jikos/junk/.config

Either later today or tomorrow I could test with the same physical start 
and align values you're using to see whether that'd make any difference.

> > As discussed with Rafael privately, I also tried this very patch
> > (x86/power/64: Always create temporary identity mapping correctly) on top
> > of the reverted revert of 021182e52fe01c1f7b1 (see the full log below),
> > but such kernel triple faults on resume as well.
> >
> > 87c38d2 x86/power/64: Always create temporary identity mapping correctly
> > 3cb504a Revert "Revert "x86/mm: Enable KASLR for physical mapping memory regions""
> > 758850d Revert "x86/mm: Enable KASLR for physical mapping memory regions"
> > 4a02dfb Revert "x86/mm: Enable KASLR for vmalloc memory regions"
> > 037863f Revert "x86/mm: Add memory hotplug support for KASLR memory randomization"
> > 3416a21 Revert "x86/mm: Do not reference phys addr beyond kernel"
> > 69227be Revert "mm: reorganize SLAB freelist randomization"
> > a1d8d71 Revert "mm: SLUB freelist randomization"
> >
> > IOW, 021182e52f introduces a bug for which there is no existing fix yet.
> 
> You mean it is something different from the previous KASLR bugs we saw?

No, I just wanted to explicitly point out that "x86/power/64: Always 
create temporary identity mapping correctly" is not a fix for this issue.

-- 
Jiri Kosina
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1458908 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina <jikos@kernel.org> wrote:
> On Tue, 9 Aug 2016, Thomas Garnier wrote:
>
>> >> Okay, I did one-by-one reverts, and the one above, i.e.
>> >>
>> >>       commit 021182e52fe01c1f7b126f97fd6ba048dc4234fd
>> >>       Author: Thomas Garnier <thgarnie@google.com>
>> >>       Date:   Tue Jun 21 17:47:03 2016 -0700
>> >>
>> >>           x86/mm: Enable KASLR for physical mapping memory regions
>> >>
>> >> is the one that is the culprit on my machine. With this one reverted,
>> >> resume hibernation doesn't reboot (tripple fault?), but proceeds
>> >> succesfully.
>>
>> My .config is attached. It is basically defconfig (x86_64) + kvmconfig
>> plus the following:
>>
>> CONFIG_PHYSICAL_START=0x1000000
>> CONFIG_RELOCATABLE=y
>> CONFIG_RANDOMIZE_BASE=y
>> CONFIG_X86_NEED_RELOCS=y
>> CONFIG_PHYSICAL_ALIGN=0x1000000
>> CONFIG_RANDOMIZE_MEMORY=y
>> CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING=0xa
>> CONFIG_X86_PTDUMP_CORE=y
>> CONFIG_X86_PTDUMP=y
>> CONFIG_KALLSYMS=y
>> CONFIG_KALLSYMS_ALL=y
>> CONFIG_KALLSYMS_ABSOLUTE_PERCPU=y
>> CONFIG_KALLSYMS_BASE_RELATIVE=y
>> CONFIG_PANIC_ON_OOPS=y
>> CONFIG_KGDB=y
>> CONFIG_EARLY_PRINTK=y
>> CONFIG_EARLY_PRINTK_DBGP=y
>> CONFIG_DEBUG_INFO=y
>> CONFIG_DEBUG_INFO_DWARF4=y
>
> The config I am reproducing the bug with (on thinkpad x200s) can be found
> at
>
>         http://www.jikos.cz/jikos/junk/.config
>
> Either later today or tomorrow I could test with the same physical start
> and align values you're using to see whether that'd make any difference.
>
>> > As discussed with Rafael privately, I also tried this very patch
>> > (x86/power/64: Always create temporary identity mapping correctly) on top
>> > of the reverted revert of 021182e52fe01c1f7b1 (see the full log below),
>> > but such kernel triple faults on resume as well.
>> >
>> > 87c38d2 x86/power/64: Always create temporary identity mapping correctly
>> > 3cb504a Revert "Revert "x86/mm: Enable KASLR for physical mapping memory regions""
>> > 758850d Revert "x86/mm: Enable KASLR for physical mapping memory regions"
>> > 4a02dfb Revert "x86/mm: Enable KASLR for vmalloc memory regions"
>> > 037863f Revert "x86/mm: Add memory hotplug support for KASLR memory randomization"
>> > 3416a21 Revert "x86/mm: Do not reference phys addr beyond kernel"
>> > 69227be Revert "mm: reorganize SLAB freelist randomization"
>> > a1d8d71 Revert "mm: SLUB freelist randomization"
>> >
>> > IOW, 021182e52f introduces a bug for which there is no existing fix yet.
>>
>> You mean it is something different from the previous KASLR bugs we saw?
>
> No, I just wanted to explicitly point out that "x86/power/64: Always
> create temporary identity mapping correctly" is not a fix for this issue.

It is better to say that the $subject patch is not sufficient to fix
it, because I'm quite confident that it is necessary for that. :-)

Without the $subject patch kernel_ident_mapping_init() makes
assumptions that simply are not met in the randomized identity mapping
base case.  Moreover, hibernation works for Thomas with $subject patch
applied, but it doesn't without it.

So there is something else that we are missing.

I have a murky suspicion, but it is really weird.  Namely, what if
restore_jump_address in set_up_temporary_text_mapping() happens to be
covered by the restore kernel's identity mapping?  Then, the image
kernel's entry point may get overwritten by something else in
core_restore_code().

But is this possible even?  Thomas?

Anyway, I'll try to reproduce this issue later today.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1458915 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, Aug 9, 2016 at 9:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina <jikos@kernel.org> wrote:
>> On Tue, 9 Aug 2016, Thomas Garnier wrote:
>>
>>> >> Okay, I did one-by-one reverts, and the one above, i.e.
>>> >>
>>> >>       commit 021182e52fe01c1f7b126f97fd6ba048dc4234fd
>>> >>       Author: Thomas Garnier <thgarnie@google.com>
>>> >>       Date:   Tue Jun 21 17:47:03 2016 -0700
>>> >>
>>> >>           x86/mm: Enable KASLR for physical mapping memory regions
>>> >>
>>> >> is the one that is the culprit on my machine. With this one reverted,
>>> >> resume hibernation doesn't reboot (tripple fault?), but proceeds
>>> >> succesfully.
>>>
>>> My .config is attached. It is basically defconfig (x86_64) + kvmconfig
>>> plus the following:
>>>
>>> CONFIG_PHYSICAL_START=0x1000000
>>> CONFIG_RELOCATABLE=y
>>> CONFIG_RANDOMIZE_BASE=y
>>> CONFIG_X86_NEED_RELOCS=y
>>> CONFIG_PHYSICAL_ALIGN=0x1000000
>>> CONFIG_RANDOMIZE_MEMORY=y
>>> CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING=0xa
>>> CONFIG_X86_PTDUMP_CORE=y
>>> CONFIG_X86_PTDUMP=y
>>> CONFIG_KALLSYMS=y
>>> CONFIG_KALLSYMS_ALL=y
>>> CONFIG_KALLSYMS_ABSOLUTE_PERCPU=y
>>> CONFIG_KALLSYMS_BASE_RELATIVE=y
>>> CONFIG_PANIC_ON_OOPS=y
>>> CONFIG_KGDB=y
>>> CONFIG_EARLY_PRINTK=y
>>> CONFIG_EARLY_PRINTK_DBGP=y
>>> CONFIG_DEBUG_INFO=y
>>> CONFIG_DEBUG_INFO_DWARF4=y
>>
>> The config I am reproducing the bug with (on thinkpad x200s) can be found
>> at
>>
>>         http://www.jikos.cz/jikos/junk/.config
>>
>> Either later today or tomorrow I could test with the same physical start
>> and align values you're using to see whether that'd make any difference.
>>
>>> > As discussed with Rafael privately, I also tried this very patch
>>> > (x86/power/64: Always create temporary identity mapping correctly) on top
>>> > of the reverted revert of 021182e52fe01c1f7b1 (see the full log below),
>>> > but such kernel triple faults on resume as well.
>>> >
>>> > 87c38d2 x86/power/64: Always create temporary identity mapping correctly
>>> > 3cb504a Revert "Revert "x86/mm: Enable KASLR for physical mapping memory regions""
>>> > 758850d Revert "x86/mm: Enable KASLR for physical mapping memory regions"
>>> > 4a02dfb Revert "x86/mm: Enable KASLR for vmalloc memory regions"
>>> > 037863f Revert "x86/mm: Add memory hotplug support for KASLR memory randomization"
>>> > 3416a21 Revert "x86/mm: Do not reference phys addr beyond kernel"
>>> > 69227be Revert "mm: reorganize SLAB freelist randomization"
>>> > a1d8d71 Revert "mm: SLUB freelist randomization"
>>> >
>>> > IOW, 021182e52f introduces a bug for which there is no existing fix yet.
>>>
>>> You mean it is something different from the previous KASLR bugs we saw?
>>
>> No, I just wanted to explicitly point out that "x86/power/64: Always
>> create temporary identity mapping correctly" is not a fix for this issue.
>
> It is better to say that the $subject patch is not sufficient to fix
> it, because I'm quite confident that it is necessary for that. :-)
>
> Without the $subject patch kernel_ident_mapping_init() makes
> assumptions that simply are not met in the randomized identity mapping
> base case.  Moreover, hibernation works for Thomas with $subject patch
> applied, but it doesn't without it.
>
> So there is something else that we are missing.
>
> I have a murky suspicion, but it is really weird.  Namely, what if
> restore_jump_address in set_up_temporary_text_mapping() happens to be
> covered by the restore kernel's identity mapping?  Then, the image
> kernel's entry point may get overwritten by something else in
> core_restore_code().
>
> But is this possible even?  Thomas?

I had a similar theory before when I was investigating the original
crash. How is it avoided even without KASLR?

Given the space for the physical memory mapping, I doubt this issue
would happen all the time though.

>
> Anyway, I'll try to reproduce this issue later today.
>
> Thanks,
> Rafael

[toc] | [prev] | [next] | [standalone]

#1459125 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, Aug 9, 2016 at 6:27 PM, Thomas Garnier <thgarnie@google.com> wrote:
> On Tue, Aug 9, 2016 at 9:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina <jikos@kernel.org> wrote:
>>> On Tue, 9 Aug 2016, Thomas Garnier wrote:
>>>
>>>> >> Okay, I did one-by-one reverts, and the one above, i.e.
>>>> >>
>>>> >>       commit 021182e52fe01c1f7b126f97fd6ba048dc4234fd
>>>> >>       Author: Thomas Garnier <thgarnie@google.com>
>>>> >>       Date:   Tue Jun 21 17:47:03 2016 -0700
>>>> >>
>>>> >>           x86/mm: Enable KASLR for physical mapping memory regions
>>>> >>
>>>> >> is the one that is the culprit on my machine. With this one reverted,
>>>> >> resume hibernation doesn't reboot (tripple fault?), but proceeds
>>>> >> succesfully.
>>>>
>>>> My .config is attached. It is basically defconfig (x86_64) + kvmconfig
>>>> plus the following:
>>>>
>>>> CONFIG_PHYSICAL_START=0x1000000
>>>> CONFIG_RELOCATABLE=y
>>>> CONFIG_RANDOMIZE_BASE=y
>>>> CONFIG_X86_NEED_RELOCS=y
>>>> CONFIG_PHYSICAL_ALIGN=0x1000000
>>>> CONFIG_RANDOMIZE_MEMORY=y
>>>> CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING=0xa
>>>> CONFIG_X86_PTDUMP_CORE=y
>>>> CONFIG_X86_PTDUMP=y
>>>> CONFIG_KALLSYMS=y
>>>> CONFIG_KALLSYMS_ALL=y
>>>> CONFIG_KALLSYMS_ABSOLUTE_PERCPU=y
>>>> CONFIG_KALLSYMS_BASE_RELATIVE=y
>>>> CONFIG_PANIC_ON_OOPS=y
>>>> CONFIG_KGDB=y
>>>> CONFIG_EARLY_PRINTK=y
>>>> CONFIG_EARLY_PRINTK_DBGP=y
>>>> CONFIG_DEBUG_INFO=y
>>>> CONFIG_DEBUG_INFO_DWARF4=y
>>>
>>> The config I am reproducing the bug with (on thinkpad x200s) can be found
>>> at
>>>
>>>         http://www.jikos.cz/jikos/junk/.config
>>>
>>> Either later today or tomorrow I could test with the same physical start
>>> and align values you're using to see whether that'd make any difference.
>>>
>>>> > As discussed with Rafael privately, I also tried this very patch
>>>> > (x86/power/64: Always create temporary identity mapping correctly) on top
>>>> > of the reverted revert of 021182e52fe01c1f7b1 (see the full log below),
>>>> > but such kernel triple faults on resume as well.
>>>> >
>>>> > 87c38d2 x86/power/64: Always create temporary identity mapping correctly
>>>> > 3cb504a Revert "Revert "x86/mm: Enable KASLR for physical mapping memory regions""
>>>> > 758850d Revert "x86/mm: Enable KASLR for physical mapping memory regions"
>>>> > 4a02dfb Revert "x86/mm: Enable KASLR for vmalloc memory regions"
>>>> > 037863f Revert "x86/mm: Add memory hotplug support for KASLR memory randomization"
>>>> > 3416a21 Revert "x86/mm: Do not reference phys addr beyond kernel"
>>>> > 69227be Revert "mm: reorganize SLAB freelist randomization"
>>>> > a1d8d71 Revert "mm: SLUB freelist randomization"
>>>> >
>>>> > IOW, 021182e52f introduces a bug for which there is no existing fix yet.
>>>>
>>>> You mean it is something different from the previous KASLR bugs we saw?
>>>
>>> No, I just wanted to explicitly point out that "x86/power/64: Always
>>> create temporary identity mapping correctly" is not a fix for this issue.
>>
>> It is better to say that the $subject patch is not sufficient to fix
>> it, because I'm quite confident that it is necessary for that. :-)
>>
>> Without the $subject patch kernel_ident_mapping_init() makes
>> assumptions that simply are not met in the randomized identity mapping
>> base case.  Moreover, hibernation works for Thomas with $subject patch
>> applied, but it doesn't without it.
>>
>> So there is something else that we are missing.
>>
>> I have a murky suspicion, but it is really weird.  Namely, what if
>> restore_jump_address in set_up_temporary_text_mapping() happens to be
>> covered by the restore kernel's identity mapping?  Then, the image
>> kernel's entry point may get overwritten by something else in
>> core_restore_code().
>>
>> But is this possible even?  Thomas?
>
> I had a similar theory before when I was investigating the original
> crash. How is it avoided even without KASLR?

It doesn't have to be actively avoided then.  restore_jump_address is
a kernel text address and if __PAGE_OFFSET is the same for both the
restore and image kernels, it is guaranteed to be above the identity
mapping in both of them.

If the base of the identity mapping is randomized in both of them,
though, that may not be guaranteed any more.

> Given the space for the physical memory mapping, I doubt this issue
> would happen all the time though.

It should not, but it's not impossible for it to happen every time, at
least in a small number of attempts.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1459091 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, 9 Aug 2016, Rafael J. Wysocki wrote:

> I have a murky suspicion, but it is really weird.  Namely, what if
> restore_jump_address in set_up_temporary_text_mapping() happens to be
> covered by the restore kernel's identity mapping?  Then, the image
> kernel's entry point may get overwritten by something else in
> core_restore_code().

So this made me to actually test a scenario where I'd suspend a kernel 
that's known-broken (i.e. contains 021182e52fe), and then have it resumed 
by a kernel that has 021182e52fe reverted. It resumed successfully.

Just a datapoint.

-- 
Jiri Kosina
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1459130 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Tue, Aug 9, 2016 at 10:02 PM, Jiri Kosina <jikos@kernel.org> wrote:
> On Tue, 9 Aug 2016, Rafael J. Wysocki wrote:
>
>> I have a murky suspicion, but it is really weird.  Namely, what if
>> restore_jump_address in set_up_temporary_text_mapping() happens to be
>> covered by the restore kernel's identity mapping?  Then, the image
>> kernel's entry point may get overwritten by something else in
>> core_restore_code().
>
> So this made me to actually test a scenario where I'd suspend a kernel
> that's known-broken (i.e. contains 021182e52fe), and then have it resumed
> by a kernel that has 021182e52fe reverted. It resumed successfully.
>
> Just a datapoint.

That indicates the problem is somewhere in the restore kernel and no
surprises there.

I am able to reproduce the original problem (a triple fault on resume
with CONFIG_RANDOMIZE_MEMORY set) without the $subject patch, but the
patch fixes it for me.

Question is why it is not sufficient for you and Boris and the above
theory is about the only one I can come up with ATM.

I'm going to compare the configs etc, but I guess I just end up
writing a patch to test that theory unless someone has any other idea
in the meantime.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1459209

On Tuesday, August 09, 2016 11:23:31 PM Rafael J. Wysocki wrote:
> On Tue, Aug 9, 2016 at 10:02 PM, Jiri Kosina <jikos@kernel.org> wrote:
> > On Tue, 9 Aug 2016, Rafael J. Wysocki wrote:
> >
> >> I have a murky suspicion, but it is really weird.  Namely, what if
> >> restore_jump_address in set_up_temporary_text_mapping() happens to be
> >> covered by the restore kernel's identity mapping?  Then, the image
> >> kernel's entry point may get overwritten by something else in
> >> core_restore_code().
> >
> > So this made me to actually test a scenario where I'd suspend a kernel
> > that's known-broken (i.e. contains 021182e52fe), and then have it resumed
> > by a kernel that has 021182e52fe reverted. It resumed successfully.
> >
> > Just a datapoint.
> 
> That indicates the problem is somewhere in the restore kernel and no
> surprises there.
> 
> I am able to reproduce the original problem (a triple fault on resume
> with CONFIG_RANDOMIZE_MEMORY set) without the $subject patch, but the
> patch fixes it for me.
> 
> Question is why it is not sufficient for you and Boris and the above
> theory is about the only one I can come up with ATM.
> 
> I'm going to compare the configs etc, but I guess I just end up
> writing a patch to test that theory unless someone has any other idea
> in the meantime.

For the lack of better ideas, below is a patch to try.

It avoids the possible issue with the restore kernel's identity mapping overlap
with restore_jump_address by creating special super-simple page tables just
for the final jump to the image kernel.

It is on top of the $subject patch.  My test box still works with this applied,
but then it worked without it as well.

If it doesn't help, the identity mapping created by set_up_temporary_mappings()
is still not adequate for some reason most likely and we'll need to find out
why.

Thanks,
Rafael


---
 arch/x86/power/hibernate_64.c     |   40 +++++++++++++++++++++++++++++++-------
 arch/x86/power/hibernate_asm_64.S |   10 +++++++++
 2 files changed, 43 insertions(+), 7 deletions(-)

Index: linux-pm/arch/x86/power/hibernate_64.c
===================================================================
--- linux-pm.orig/arch/x86/power/hibernate_64.c
+++ linux-pm/arch/x86/power/hibernate_64.c
@@ -38,14 +38,20 @@ unsigned long jump_address_phys;
 unsigned long restore_cr3 __visible;
 
 unsigned long temp_level4_pgt __visible;
+unsigned long jump_level4_pgt __visible;
 
 unsigned long relocated_restore_code __visible;
 
-static int set_up_temporary_text_mapping(pgd_t *pgd)
+static int set_up_temporary_text_mapping(void)
 {
+	pgd_t *pgd;
 	pmd_t *pmd;
 	pud_t *pud;
 
+	pgd = (pgd_t *)get_safe_page(GFP_ATOMIC);
+	if (!pgd)
+		return -ENOMEM;
+
 	/*
 	 * The new mapping only has to cover the page containing the image
 	 * kernel's entry point (jump_address_phys), because the switch over to
@@ -74,6 +80,23 @@ static int set_up_temporary_text_mapping
 	set_pgd(pgd + pgd_index(restore_jump_address),
 		__pgd(__pa(pud) | _KERNPG_TABLE));
 
+	pud = (pud_t *)get_safe_page(GFP_ATOMIC);
+	if (!pud)
+		return -ENOMEM;
+
+	pmd = (pmd_t *)get_safe_page(GFP_ATOMIC);
+	if (!pmd)
+		return -ENOMEM;
+
+	set_pmd(pmd + pmd_index(relocated_restore_code),
+		__pmd((__pa(relocated_restore_code) & PMD_MASK) | __PAGE_KERNEL_LARGE_EXEC));
+	set_pud(pud + pud_index(relocated_restore_code),
+		__pud(__pa(pmd) | _KERNPG_TABLE));
+	set_pgd(pgd + pgd_index(relocated_restore_code),
+		__pgd(__pa(pud) | _KERNPG_TABLE));
+
+	jump_level4_pgt = __pa(pgd);
+
 	return 0;
 }
 
@@ -98,11 +121,6 @@ static int set_up_temporary_mappings(voi
 	if (!pgd)
 		return -ENOMEM;
 
-	/* Prepare a temporary mapping for the kernel text */
-	result = set_up_temporary_text_mapping(pgd);
-	if (result)
-		return result;
-
 	/* Set up the direct mapping from scratch */
 	for (i = 0; i < nr_pfn_mapped; i++) {
 		mstart = pfn_mapped[i].start << PAGE_SHIFT;
@@ -122,7 +140,10 @@ static int relocate_restore_code(void)
 	pgd_t *pgd;
 	pud_t *pud;
 
-	relocated_restore_code = get_safe_page(GFP_ATOMIC);
+	do
+		relocated_restore_code = get_safe_page(GFP_ATOMIC);
+	while ((relocated_restore_code & PMD_MASK) == (restore_jump_address & PMD_MASK));
+
 	if (!relocated_restore_code)
 		return -ENOMEM;
 
@@ -162,6 +183,11 @@ int swsusp_arch_resume(void)
 	if (error)
 		return error;
 
+	/* Prepare a temporary mapping for the jump to the image kernel */
+	error = set_up_temporary_text_mapping();
+	if (error)
+		return error;
+
 	restore_image();
 	return 0;
 }
Index: linux-pm/arch/x86/power/hibernate_asm_64.S
===================================================================
--- linux-pm.orig/arch/x86/power/hibernate_asm_64.S
+++ linux-pm/arch/x86/power/hibernate_asm_64.S
@@ -57,6 +57,7 @@ ENTRY(restore_image)
 	/* prepare to jump to the image kernel */
 	movq	restore_jump_address(%rip), %r8
 	movq	restore_cr3(%rip), %r9
+	movq	jump_level4_pgt(%rip), %r10
 
 	/* prepare to switch to temporary page tables */
 	movq	temp_level4_pgt(%rip), %rax
@@ -96,6 +97,15 @@ ENTRY(core_restore_code)
 	jmp	.Lloop
 
 .Ldone:
+	/* switch to jump page tables */
+	movq	%r10, %cr3
+	/* flush TLB */
+	movq	%rbx, %rcx
+	andq	$~(X86_CR4_PGE), %rcx
+	movq	%rcx, %cr4;  # turn off PGE
+	movq	%cr3, %rcx;  # flush TLB
+	movq	%rcx, %cr3;
+	movq	%rbx, %cr4;  # turn PGE back on
 	/* jump to the restore_registers address from the image header */
 	jmpq	*%r8
 

[toc] | [prev] | [next] | [standalone]

#1459474 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Wed, Aug 10, 2016 at 9:35 AM, Borislav Petkov <bp@suse.de> wrote:
> On Wed, Aug 10, 2016 at 04:59:40PM +0200, Jiri Kosina wrote:
>> Mine is Lenovo thinkpad x200s; I think Boris has been testing it on x230s,
>
> It says "X230" here under the screen.
>
>> but not sure whether any of the latest patches didn't actually fix it for
>> him.
>
> Haven't tested them yet. I'm waiting for you to test them first since
> this is the only machine I have right now and I need it for work.
>
>> The machine I am seeing the issue on, has 2G RAM, with this e820 map:
>
> 8G here:
>
> e820: BIOS-provided physical RAM map:
> BIOS-e820: [mem 0x0000000000000000-0x000000000009d7ff] usable
> BIOS-e820: [mem 0x000000000009d800-0x000000000009ffff] reserved
> BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
> BIOS-e820: [mem 0x0000000000100000-0x000000001fffffff] usable
> BIOS-e820: [mem 0x0000000020000000-0x00000000201fffff] reserved
> BIOS-e820: [mem 0x0000000020200000-0x0000000040003fff] usable
> BIOS-e820: [mem 0x0000000040004000-0x0000000040004fff] reserved
> BIOS-e820: [mem 0x0000000040005000-0x00000000cec2ffff] usable
> BIOS-e820: [mem 0x00000000cec30000-0x00000000dae9efff] reserved
> BIOS-e820: [mem 0x00000000dae9f000-0x00000000daf9efff] ACPI NVS
> BIOS-e820: [mem 0x00000000daf9f000-0x00000000daffefff] ACPI data
> BIOS-e820: [mem 0x00000000dafff000-0x00000000df9fffff] reserved
> BIOS-e820: [mem 0x00000000f8000000-0x00000000fbffffff] reserved
> BIOS-e820: [mem 0x00000000fec00000-0x00000000fec00fff] reserved
> BIOS-e820: [mem 0x00000000fed08000-0x00000000fed08fff] reserved
> BIOS-e820: [mem 0x00000000fed10000-0x00000000fed19fff] reserved
> BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
> BIOS-e820: [mem 0x00000000fee00000-0x00000000fee00fff] reserved
> BIOS-e820: [mem 0x00000000ffc00000-0x00000000ffffffff] reserved
> BIOS-e820: [mem 0x0000000100000000-0x000000021e5fffff] usable
> BIOS-e820: [mem 0x000000021e600000-0x000000021e7fffff] reserved
> debug: ignoring loglevel setting.
> NX (Execute Disable) protection: active
> SMBIOS 2.7 present.
> DMI: LENOVO 2320CTO/2320CTO, BIOS G2ET86WW (2.06 ) 11/13/2012
> e820: update [mem 0x00000000-0x00000fff] usable ==> reserved
> e820: remove [mem 0x000a0000-0x000fffff] usable
> e820: last_pfn = 0x21e600 max_arch_pfn = 0x400000000
>
>> CPU:
>
> processor       : 0
> vendor_id       : GenuineIntel
> cpu family      : 6
> model           : 58
> model name      : Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz
> stepping        : 9
> microcode       : 0x1c
> cpu MHz         : 1257.421
> cache size      : 4096 KB
> physical id     : 0
> siblings        : 4
> core id         : 0
> cpu cores       : 2
> apicid          : 0
> initial apicid  : 0
> fpu             : yes
> fpu_exception   : yes
> cpuid level     : 13
> wp              : yes
> flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm epb tpr_shadow vnmi flexpriority ept vpid fsgsbase smep erms xsaveopt dtherm ida arat pln pts
> bugs            :
> bogomips        : 5786.68
> clflush size    : 64
> cache_alignment : 64
> address sizes   : 36 bits physical, 48 bits virtual
> power management:
>

Ok, I want to know if the problem is the PUD alignment or the change
of PAGE_OFFSET based all together. Can you test the following change?
(on top of everything else with KASLR enabled). It will randomize the
memory sections only on PGD level.

diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index ec8654f..a8477b0 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -120,7 +120,7 @@ void __init kernel_randomize_memory(void)
                 */
                entropy = remain_entropy / (ARRAY_SIZE(kaslr_regions) - i);
                prandom_bytes_state(&rand_state, &rand, sizeof(rand));
-               entropy = (rand % (entropy + 1)) & PUD_MASK;
+               entropy = (rand % (entropy + 1)) & PGDIR_MASK;
                vaddr += entropy;
                *kaslr_regions[i].base = vaddr;

@@ -129,7 +129,7 @@ void __init kernel_randomize_memory(void)
                 * randomization alignment.
                 */
                vaddr += get_padding(&kaslr_regions[i]);
-               vaddr = round_up(vaddr + 1, PUD_SIZE);
+               vaddr = round_up(vaddr + 1, PGDIR_SIZE);
                remain_entropy -= entropy;
        }
 }

> --
> Regards/Gruss,
>     Boris.
>
> ECO tip #101: Trim your mails when you reply.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> --

[toc] | [prev] | [next] | [standalone]

#1459534 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:

> For the lack of better ideas, below is a patch to try.
> 
> It avoids the possible issue with the restore kernel's identity mapping overlap
> with restore_jump_address by creating special super-simple page tables just
> for the final jump to the image kernel.
> 
> It is on top of the $subject patch.  My test box still works with this applied,
> but then it worked without it as well.
> 
> If it doesn't help, the identity mapping created by set_up_temporary_mappings()
> is still not adequate for some reason most likely and we'll need to find out
> why.

Unfortunately, still with $subject patch + this one, triple fault and 
reboot after reading the hibernation image.

Due to being slightly out of ideas currently, I'll play a little bit more 
with the relocation offsets to see whether that makes any difference.

-- 
Jiri Kosina
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1459541 — Re: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly

On Wed, Aug 10, 2016 at 04:59:40PM +0200, Jiri Kosina wrote:
> Mine is Lenovo thinkpad x200s; I think Boris has been testing it on x230s,

It says "X230" here under the screen.

> but not sure whether any of the latest patches didn't actually fix it for 
> him.

Haven't tested them yet. I'm waiting for you to test them first since
this is the only machine I have right now and I need it for work.

> The machine I am seeing the issue on, has 2G RAM, with this e820 map:

8G here:

e820: BIOS-provided physical RAM map:
BIOS-e820: [mem 0x0000000000000000-0x000000000009d7ff] usable
BIOS-e820: [mem 0x000000000009d800-0x000000000009ffff] reserved
BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
BIOS-e820: [mem 0x0000000000100000-0x000000001fffffff] usable
BIOS-e820: [mem 0x0000000020000000-0x00000000201fffff] reserved
BIOS-e820: [mem 0x0000000020200000-0x0000000040003fff] usable
BIOS-e820: [mem 0x0000000040004000-0x0000000040004fff] reserved
BIOS-e820: [mem 0x0000000040005000-0x00000000cec2ffff] usable
BIOS-e820: [mem 0x00000000cec30000-0x00000000dae9efff] reserved
BIOS-e820: [mem 0x00000000dae9f000-0x00000000daf9efff] ACPI NVS
BIOS-e820: [mem 0x00000000daf9f000-0x00000000daffefff] ACPI data
BIOS-e820: [mem 0x00000000dafff000-0x00000000df9fffff] reserved
BIOS-e820: [mem 0x00000000f8000000-0x00000000fbffffff] reserved
BIOS-e820: [mem 0x00000000fec00000-0x00000000fec00fff] reserved
BIOS-e820: [mem 0x00000000fed08000-0x00000000fed08fff] reserved
BIOS-e820: [mem 0x00000000fed10000-0x00000000fed19fff] reserved
BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
BIOS-e820: [mem 0x00000000fee00000-0x00000000fee00fff] reserved
BIOS-e820: [mem 0x00000000ffc00000-0x00000000ffffffff] reserved
BIOS-e820: [mem 0x0000000100000000-0x000000021e5fffff] usable
BIOS-e820: [mem 0x000000021e600000-0x000000021e7fffff] reserved
debug: ignoring loglevel setting.
NX (Execute Disable) protection: active
SMBIOS 2.7 present.
DMI: LENOVO 2320CTO/2320CTO, BIOS G2ET86WW (2.06 ) 11/13/2012
e820: update [mem 0x00000000-0x00000fff] usable ==> reserved
e820: remove [mem 0x000a0000-0x000fffff] usable
e820: last_pfn = 0x21e600 max_arch_pfn = 0x400000000

> CPU:

processor       : 0
vendor_id       : GenuineIntel
cpu family      : 6
model           : 58
model name      : Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz
stepping        : 9
microcode       : 0x1c
cpu MHz         : 1257.421
cache size      : 4096 KB
physical id     : 0
siblings        : 4
core id         : 0
cpu cores       : 2
apicid          : 0
initial apicid  : 0
fpu             : yes
fpu_exception   : yes
cpuid level     : 13
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm epb tpr_shadow vnmi flexpriority ept vpid fsgsbase smep erms xsaveopt dtherm ida arat pln pts
bugs            :
bogomips        : 5786.68
clflush size    : 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
power management:

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web