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

[Thomas Garnier]

On Thu, Aug 11, 2016 at 2:33 PM, Rafael J. Wysocki  wrote:
> On Thursday, August 11, 2016 11:47:27 AM Thomas Garnier wrote:
>> On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki  wrote:
>> > On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier  
>> > wrote:
>> >> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  
>> >> wrote:
>> >>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
>>  On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>> 
>> > So I used your .config to generate one for my test machine and with
>> > that I can reproduce.
>> 
>>  Was that the config I've sent, or did Boris provide one as well? Which 
>>  one
>>  are you able to reproduce with please?
>> >>>
>> >>> It's the Boris' one.
>> >>>
>> >>> Moreover, I have found the options that make the difference: unsetting
>> >>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
>> >>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
>> >>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
>> >>>
>> >>> Unbelievable, but that's what I'm seeing.
>> >>
>> >> Nice find!
>> >>
>> >>>
>> >>> Now, that leads to a few questions:
>> >>>
>> >>> - How does lockdep change the picture so it matters for hibernation?
>> >>> - Why is hibernation the only piece that's affected?
>> >>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
>> >>>
>> >>> Thomas, any ideas?
>> >>
>> >> No idea so far. I will investigate though.
>> >>
>> >> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
>> >> don't think it was related because it was on early boot and with
>> >> certain e820 memory layout (and PUD randomization that I disabled on
>> >> the previous patch test). The fix is on tip:
>> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d
>> >
>> > Well, I don't think this is related.
>> >
>> > In the meantime, I went back to my original .config and verified that
>> > setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with
>> > CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so
>> > this really matters somehow.
>> >
>> > Besides, now that I have a reproducer, I can check various other
>> > things and for example this change (sorry for broken whitespace):
>> >
>> > Index: linux-pm/arch/x86/mm/kaslr.c
>> > ===
>> > --- linux-pm.orig/arch/x86/mm/kaslr.c
>> > +++ linux-pm/arch/x86/mm/kaslr.c
>> > @@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void
>> >  prandom_bytes_state(_state, , sizeof(rand));
>> >  entropy = (rand % (entropy + 1)) & PUD_MASK;
>> >  vaddr += entropy;
>> > -*kaslr_regions[i].base = vaddr;
>> > +*kaslr_regions[i].base += PUD_SIZE;
>> >
>>
>> I think it works because the address is fixed now (just PUD aligned).
>
> That's exactly right.
>
>> >  /*
>> >   * Jump the region and add a minimum padding based on
>> >
>> > makes hibernation work for me again in the above configuration.  To
>> > me, this means that the $subject patch works as expected and the
>> > problem really is related to the vaddr value being too big.
>> >
>>
>> I managed to debug the restoration and found that a first access
>> violation happens here:
>
> So you were able to get a bit farther than I did. :-)
>
>> (gdb) x/20i 0xb20a46de
>>0xb20a46de :
>> moveax,DWORD PTR gs:[rip+0x4df65a4b]# 0xa130 
>>
>> Handled by do_async_page_fault which will fault as well on this instruction:
>>
>> => 0xb2047ca1 :
>> moveax,DWORD PTR gs:[rip+0x4dfc4e58]# 0xcb00 
>>
>> So there is a problem with the gs register not being restored
>> correctly at this stage.
>>
>> In create_image, there is tracing (trace_suspend_resume) before and
>> after the suspend. Except at this stage, gs was not yet restored. It
>> uses the old gs leading to the double fault.
>
> Nice catch!

Thanks, got lucky.

>
> I established that the problem happened when there was a difference between
> the page_offset_base values in the restore and image kernels, so my conclusion
> was that the code leaked some information related to virtual addresses from
> the restore kernel back to the mage one.  Which is exactly what you found. :-)
>
>> I tested this fix to be correct:
>>
>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>> index a881c6a..33c79b6 100644
>> --- a/kernel/power/hibernate.c
>> +++ b/kernel/power/hibernate.c
>> @@ -300,12 +300,12 @@ static int create_image(int platform_mode)
>>   save_processor_state();
>>   trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true);
>>   error = swsusp_arch_suspend();
>> + /* Restore control flow magically appears 

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

[Thomas Garnier]

On Thu, Aug 11, 2016 at 2:33 PM, Rafael J. Wysocki  wrote:
> On Thursday, August 11, 2016 11:47:27 AM Thomas Garnier wrote:
>> On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki  wrote:
>> > On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier  
>> > wrote:
>> >> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  
>> >> wrote:
>> >>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
>>  On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>> 
>> > So I used your .config to generate one for my test machine and with
>> > that I can reproduce.
>> 
>>  Was that the config I've sent, or did Boris provide one as well? Which 
>>  one
>>  are you able to reproduce with please?
>> >>>
>> >>> It's the Boris' one.
>> >>>
>> >>> Moreover, I have found the options that make the difference: unsetting
>> >>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
>> >>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
>> >>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
>> >>>
>> >>> Unbelievable, but that's what I'm seeing.
>> >>
>> >> Nice find!
>> >>
>> >>>
>> >>> Now, that leads to a few questions:
>> >>>
>> >>> - How does lockdep change the picture so it matters for hibernation?
>> >>> - Why is hibernation the only piece that's affected?
>> >>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
>> >>>
>> >>> Thomas, any ideas?
>> >>
>> >> No idea so far. I will investigate though.
>> >>
>> >> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
>> >> don't think it was related because it was on early boot and with
>> >> certain e820 memory layout (and PUD randomization that I disabled on
>> >> the previous patch test). The fix is on tip:
>> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d
>> >
>> > Well, I don't think this is related.
>> >
>> > In the meantime, I went back to my original .config and verified that
>> > setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with
>> > CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so
>> > this really matters somehow.
>> >
>> > Besides, now that I have a reproducer, I can check various other
>> > things and for example this change (sorry for broken whitespace):
>> >
>> > Index: linux-pm/arch/x86/mm/kaslr.c
>> > ===
>> > --- linux-pm.orig/arch/x86/mm/kaslr.c
>> > +++ linux-pm/arch/x86/mm/kaslr.c
>> > @@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void
>> >  prandom_bytes_state(_state, , sizeof(rand));
>> >  entropy = (rand % (entropy + 1)) & PUD_MASK;
>> >  vaddr += entropy;
>> > -*kaslr_regions[i].base = vaddr;
>> > +*kaslr_regions[i].base += PUD_SIZE;
>> >
>>
>> I think it works because the address is fixed now (just PUD aligned).
>
> That's exactly right.
>
>> >  /*
>> >   * Jump the region and add a minimum padding based on
>> >
>> > makes hibernation work for me again in the above configuration.  To
>> > me, this means that the $subject patch works as expected and the
>> > problem really is related to the vaddr value being too big.
>> >
>>
>> I managed to debug the restoration and found that a first access
>> violation happens here:
>
> So you were able to get a bit farther than I did. :-)
>
>> (gdb) x/20i 0xb20a46de
>>0xb20a46de :
>> moveax,DWORD PTR gs:[rip+0x4df65a4b]# 0xa130 
>>
>> Handled by do_async_page_fault which will fault as well on this instruction:
>>
>> => 0xb2047ca1 :
>> moveax,DWORD PTR gs:[rip+0x4dfc4e58]# 0xcb00 
>>
>> So there is a problem with the gs register not being restored
>> correctly at this stage.
>>
>> In create_image, there is tracing (trace_suspend_resume) before and
>> after the suspend. Except at this stage, gs was not yet restored. It
>> uses the old gs leading to the double fault.
>
> Nice catch!

Thanks, got lucky.

>
> I established that the problem happened when there was a difference between
> the page_offset_base values in the restore and image kernels, so my conclusion
> was that the code leaked some information related to virtual addresses from
> the restore kernel back to the mage one.  Which is exactly what you found. :-)
>
>> I tested this fix to be correct:
>>
>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>> index a881c6a..33c79b6 100644
>> --- a/kernel/power/hibernate.c
>> +++ b/kernel/power/hibernate.c
>> @@ -300,12 +300,12 @@ static int create_image(int platform_mode)
>>   save_processor_state();
>>   trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true);
>>   error = swsusp_arch_suspend();
>> + /* Restore control flow magically appears here */
>> + restore_processor_state();
>>   trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false);
>>   if (error)
>>   printk(KERN_ERR "PM: 

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

[Rafael J. Wysocki]

On Thursday, August 11, 2016 11:47:27 AM Thomas Garnier wrote:
> On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki  wrote:
> > On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier  wrote:
> >> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  
> >> wrote:
> >>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
>  On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
> 
> > So I used your .config to generate one for my test machine and with
> > that I can reproduce.
> 
>  Was that the config I've sent, or did Boris provide one as well? Which 
>  one
>  are you able to reproduce with please?
> >>>
> >>> It's the Boris' one.
> >>>
> >>> Moreover, I have found the options that make the difference: unsetting
> >>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
> >>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
> >>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
> >>>
> >>> Unbelievable, but that's what I'm seeing.
> >>
> >> Nice find!
> >>
> >>>
> >>> Now, that leads to a few questions:
> >>>
> >>> - How does lockdep change the picture so it matters for hibernation?
> >>> - Why is hibernation the only piece that's affected?
> >>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
> >>>
> >>> Thomas, any ideas?
> >>
> >> No idea so far. I will investigate though.
> >>
> >> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
> >> don't think it was related because it was on early boot and with
> >> certain e820 memory layout (and PUD randomization that I disabled on
> >> the previous patch test). The fix is on tip:
> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d
> >
> > Well, I don't think this is related.
> >
> > In the meantime, I went back to my original .config and verified that
> > setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with
> > CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so
> > this really matters somehow.
> >
> > Besides, now that I have a reproducer, I can check various other
> > things and for example this change (sorry for broken whitespace):
> >
> > Index: linux-pm/arch/x86/mm/kaslr.c
> > ===
> > --- linux-pm.orig/arch/x86/mm/kaslr.c
> > +++ linux-pm/arch/x86/mm/kaslr.c
> > @@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void
> >  prandom_bytes_state(_state, , sizeof(rand));
> >  entropy = (rand % (entropy + 1)) & PUD_MASK;
> >  vaddr += entropy;
> > -*kaslr_regions[i].base = vaddr;
> > +*kaslr_regions[i].base += PUD_SIZE;
> >
> 
> I think it works because the address is fixed now (just PUD aligned).

That's exactly right.

> >  /*
> >   * Jump the region and add a minimum padding based on
> >
> > makes hibernation work for me again in the above configuration.  To
> > me, this means that the $subject patch works as expected and the
> > problem really is related to the vaddr value being too big.
> >
> 
> I managed to debug the restoration and found that a first access
> violation happens here:

So you were able to get a bit farther than I did. :-)

> (gdb) x/20i 0xb20a46de
>0xb20a46de :
> moveax,DWORD PTR gs:[rip+0x4df65a4b]# 0xa130 
> 
> Handled by do_async_page_fault which will fault as well on this instruction:
> 
> => 0xb2047ca1 :
> moveax,DWORD PTR gs:[rip+0x4dfc4e58]# 0xcb00 
> 
> So there is a problem with the gs register not being restored
> correctly at this stage.
> 
> In create_image, there is tracing (trace_suspend_resume) before and
> after the suspend. Except at this stage, gs was not yet restored. It
> uses the old gs leading to the double fault.

Nice catch!

I established that the problem happened when there was a difference between
the page_offset_base values in the restore and image kernels, so my conclusion
was that the code leaked some information related to virtual addresses from
the restore kernel back to the mage one.  Which is exactly what you found. :-)

> I tested this fix to be correct:
> 
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index a881c6a..33c79b6 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -300,12 +300,12 @@ static int create_image(int platform_mode)
>   save_processor_state();
>   trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true);
>   error = swsusp_arch_suspend();
> + /* Restore control flow magically appears here */
> + restore_processor_state();
>   trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false);
>   if (error)
>   printk(KERN_ERR "PM: Error %d creating hibernation image\n",
>   error);
> - /* Restore 

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

[Rafael J. Wysocki]

On Thursday, August 11, 2016 11:47:27 AM Thomas Garnier wrote:
> On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki  wrote:
> > On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier  wrote:
> >> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  
> >> wrote:
> >>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
>  On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
> 
> > So I used your .config to generate one for my test machine and with
> > that I can reproduce.
> 
>  Was that the config I've sent, or did Boris provide one as well? Which 
>  one
>  are you able to reproduce with please?
> >>>
> >>> It's the Boris' one.
> >>>
> >>> Moreover, I have found the options that make the difference: unsetting
> >>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
> >>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
> >>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
> >>>
> >>> Unbelievable, but that's what I'm seeing.
> >>
> >> Nice find!
> >>
> >>>
> >>> Now, that leads to a few questions:
> >>>
> >>> - How does lockdep change the picture so it matters for hibernation?
> >>> - Why is hibernation the only piece that's affected?
> >>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
> >>>
> >>> Thomas, any ideas?
> >>
> >> No idea so far. I will investigate though.
> >>
> >> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
> >> don't think it was related because it was on early boot and with
> >> certain e820 memory layout (and PUD randomization that I disabled on
> >> the previous patch test). The fix is on tip:
> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d
> >
> > Well, I don't think this is related.
> >
> > In the meantime, I went back to my original .config and verified that
> > setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with
> > CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so
> > this really matters somehow.
> >
> > Besides, now that I have a reproducer, I can check various other
> > things and for example this change (sorry for broken whitespace):
> >
> > Index: linux-pm/arch/x86/mm/kaslr.c
> > ===
> > --- linux-pm.orig/arch/x86/mm/kaslr.c
> > +++ linux-pm/arch/x86/mm/kaslr.c
> > @@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void
> >  prandom_bytes_state(_state, , sizeof(rand));
> >  entropy = (rand % (entropy + 1)) & PUD_MASK;
> >  vaddr += entropy;
> > -*kaslr_regions[i].base = vaddr;
> > +*kaslr_regions[i].base += PUD_SIZE;
> >
> 
> I think it works because the address is fixed now (just PUD aligned).

That's exactly right.

> >  /*
> >   * Jump the region and add a minimum padding based on
> >
> > makes hibernation work for me again in the above configuration.  To
> > me, this means that the $subject patch works as expected and the
> > problem really is related to the vaddr value being too big.
> >
> 
> I managed to debug the restoration and found that a first access
> violation happens here:

So you were able to get a bit farther than I did. :-)

> (gdb) x/20i 0xb20a46de
>0xb20a46de :
> moveax,DWORD PTR gs:[rip+0x4df65a4b]# 0xa130 
> 
> Handled by do_async_page_fault which will fault as well on this instruction:
> 
> => 0xb2047ca1 :
> moveax,DWORD PTR gs:[rip+0x4dfc4e58]# 0xcb00 
> 
> So there is a problem with the gs register not being restored
> correctly at this stage.
> 
> In create_image, there is tracing (trace_suspend_resume) before and
> after the suspend. Except at this stage, gs was not yet restored. It
> uses the old gs leading to the double fault.

Nice catch!

I established that the problem happened when there was a difference between
the page_offset_base values in the restore and image kernels, so my conclusion
was that the code leaked some information related to virtual addresses from
the restore kernel back to the mage one.  Which is exactly what you found. :-)

> I tested this fix to be correct:
> 
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index a881c6a..33c79b6 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -300,12 +300,12 @@ static int create_image(int platform_mode)
>   save_processor_state();
>   trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true);
>   error = swsusp_arch_suspend();
> + /* Restore control flow magically appears here */
> + restore_processor_state();
>   trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false);
>   if (error)
>   printk(KERN_ERR "PM: Error %d creating hibernation image\n",
>   error);
> - /* Restore control flow magically appears here */
> - restore_processor_state();
>   if (!in_suspend)
>   events_check_enabled = false;
> 
> Let me know 

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

[Thomas Garnier]

On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki  wrote:
> On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier  wrote:
>> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  wrote:
>>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
 On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:

> So I used your .config to generate one for my test machine and with
> that I can reproduce.

 Was that the config I've sent, or did Boris provide one as well? Which one
 are you able to reproduce with please?
>>>
>>> It's the Boris' one.
>>>
>>> Moreover, I have found the options that make the difference: unsetting
>>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
>>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
>>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
>>>
>>> Unbelievable, but that's what I'm seeing.
>>
>> Nice find!
>>
>>>
>>> Now, that leads to a few questions:
>>>
>>> - How does lockdep change the picture so it matters for hibernation?
>>> - Why is hibernation the only piece that's affected?
>>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
>>>
>>> Thomas, any ideas?
>>
>> No idea so far. I will investigate though.
>>
>> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
>> don't think it was related because it was on early boot and with
>> certain e820 memory layout (and PUD randomization that I disabled on
>> the previous patch test). The fix is on tip:
>> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d
>
> Well, I don't think this is related.
>
> In the meantime, I went back to my original .config and verified that
> setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with
> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so
> this really matters somehow.
>
> Besides, now that I have a reproducer, I can check various other
> things and for example this change (sorry for broken whitespace):
>
> Index: linux-pm/arch/x86/mm/kaslr.c
> ===
> --- linux-pm.orig/arch/x86/mm/kaslr.c
> +++ linux-pm/arch/x86/mm/kaslr.c
> @@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void
>  prandom_bytes_state(_state, , sizeof(rand));
>  entropy = (rand % (entropy + 1)) & PUD_MASK;
>  vaddr += entropy;
> -*kaslr_regions[i].base = vaddr;
> +*kaslr_regions[i].base += PUD_SIZE;
>

I think it works because the address is fixed now (just PUD aligned).

>  /*
>   * Jump the region and add a minimum padding based on
>
> makes hibernation work for me again in the above configuration.  To
> me, this means that the $subject patch works as expected and the
> problem really is related to the vaddr value being too big.
>

I managed to debug the restoration and found that a first access
violation happens here:

(gdb) x/20i 0xb20a46de
   0xb20a46de :
moveax,DWORD PTR gs:[rip+0x4df65a4b]# 0xa130 

Handled by do_async_page_fault which will fault as well on this instruction:

=> 0xb2047ca1 :
moveax,DWORD PTR gs:[rip+0x4dfc4e58]# 0xcb00 

So there is a problem with the gs register not being restored
correctly at this stage.

In create_image, there is tracing (trace_suspend_resume) before and
after the suspend. Except at this stage, gs was not yet restored. It
uses the old gs leading to the double fault.

I tested this fix to be correct:

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index a881c6a..33c79b6 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -300,12 +300,12 @@ static int create_image(int platform_mode)
  save_processor_state();
  trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true);
  error = swsusp_arch_suspend();
+ /* Restore control flow magically appears here */
+ restore_processor_state();
  trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false);
  if (error)
  printk(KERN_ERR "PM: Error %d creating hibernation image\n",
  error);
- /* Restore control flow magically appears here */
- restore_processor_state();
  if (!in_suspend)
  events_check_enabled = false;

Let me know if it works for you. Note that I don't know why this issue
popup with the different config.

> Thanks,
> Rafael

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

[Thomas Garnier]

On Wed, Aug 10, 2016 at 6:35 PM, Rafael J. Wysocki  wrote:
> On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier  wrote:
>> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  wrote:
>>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
 On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:

> So I used your .config to generate one for my test machine and with
> that I can reproduce.

 Was that the config I've sent, or did Boris provide one as well? Which one
 are you able to reproduce with please?
>>>
>>> It's the Boris' one.
>>>
>>> Moreover, I have found the options that make the difference: unsetting
>>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
>>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
>>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
>>>
>>> Unbelievable, but that's what I'm seeing.
>>
>> Nice find!
>>
>>>
>>> Now, that leads to a few questions:
>>>
>>> - How does lockdep change the picture so it matters for hibernation?
>>> - Why is hibernation the only piece that's affected?
>>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
>>>
>>> Thomas, any ideas?
>>
>> No idea so far. I will investigate though.
>>
>> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
>> don't think it was related because it was on early boot and with
>> certain e820 memory layout (and PUD randomization that I disabled on
>> the previous patch test). The fix is on tip:
>> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d
>
> Well, I don't think this is related.
>
> In the meantime, I went back to my original .config and verified that
> setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with
> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so
> this really matters somehow.
>
> Besides, now that I have a reproducer, I can check various other
> things and for example this change (sorry for broken whitespace):
>
> Index: linux-pm/arch/x86/mm/kaslr.c
> ===
> --- linux-pm.orig/arch/x86/mm/kaslr.c
> +++ linux-pm/arch/x86/mm/kaslr.c
> @@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void
>  prandom_bytes_state(_state, , sizeof(rand));
>  entropy = (rand % (entropy + 1)) & PUD_MASK;
>  vaddr += entropy;
> -*kaslr_regions[i].base = vaddr;
> +*kaslr_regions[i].base += PUD_SIZE;
>

I think it works because the address is fixed now (just PUD aligned).

>  /*
>   * Jump the region and add a minimum padding based on
>
> makes hibernation work for me again in the above configuration.  To
> me, this means that the $subject patch works as expected and the
> problem really is related to the vaddr value being too big.
>

I managed to debug the restoration and found that a first access
violation happens here:

(gdb) x/20i 0xb20a46de
   0xb20a46de :
moveax,DWORD PTR gs:[rip+0x4df65a4b]# 0xa130 

Handled by do_async_page_fault which will fault as well on this instruction:

=> 0xb2047ca1 :
moveax,DWORD PTR gs:[rip+0x4dfc4e58]# 0xcb00 

So there is a problem with the gs register not being restored
correctly at this stage.

In create_image, there is tracing (trace_suspend_resume) before and
after the suspend. Except at this stage, gs was not yet restored. It
uses the old gs leading to the double fault.

I tested this fix to be correct:

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index a881c6a..33c79b6 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -300,12 +300,12 @@ static int create_image(int platform_mode)
  save_processor_state();
  trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, true);
  error = swsusp_arch_suspend();
+ /* Restore control flow magically appears here */
+ restore_processor_state();
  trace_suspend_resume(TPS("machine_suspend"), PM_EVENT_HIBERNATE, false);
  if (error)
  printk(KERN_ERR "PM: Error %d creating hibernation image\n",
  error);
- /* Restore control flow magically appears here */
- restore_processor_state();
  if (!in_suspend)
  events_check_enabled = false;

Let me know if it works for you. Note that I don't know why this issue
popup with the different config.

> Thanks,
> Rafael

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

[Rafael J. Wysocki]

On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier  wrote:
> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  wrote:
>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
>>> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>>>
 So I used your .config to generate one for my test machine and with
 that I can reproduce.
>>>
>>> Was that the config I've sent, or did Boris provide one as well? Which one
>>> are you able to reproduce with please?
>>
>> It's the Boris' one.
>>
>> Moreover, I have found the options that make the difference: unsetting
>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
>>
>> Unbelievable, but that's what I'm seeing.
>
> Nice find!
>
>>
>> Now, that leads to a few questions:
>>
>> - How does lockdep change the picture so it matters for hibernation?
>> - Why is hibernation the only piece that's affected?
>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
>>
>> Thomas, any ideas?
>
> No idea so far. I will investigate though.
>
> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
> don't think it was related because it was on early boot and with
> certain e820 memory layout (and PUD randomization that I disabled on
> the previous patch test). The fix is on tip:
> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d

Well, I don't think this is related.

In the meantime, I went back to my original .config and verified that
setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with
CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so
this really matters somehow.

Besides, now that I have a reproducer, I can check various other
things and for example this change (sorry for broken whitespace):

Index: linux-pm/arch/x86/mm/kaslr.c
===
--- linux-pm.orig/arch/x86/mm/kaslr.c
+++ linux-pm/arch/x86/mm/kaslr.c
@@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void
 prandom_bytes_state(_state, , sizeof(rand));
 entropy = (rand % (entropy + 1)) & PUD_MASK;
 vaddr += entropy;
-*kaslr_regions[i].base = vaddr;
+*kaslr_regions[i].base += PUD_SIZE;

 /*
  * Jump the region and add a minimum padding based on

makes hibernation work for me again in the above configuration.  To
me, this means that the $subject patch works as expected and the
problem really is related to the vaddr value being too big.

Thanks,
Rafael

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

[Rafael J. Wysocki]

On Thu, Aug 11, 2016 at 3:17 AM, Thomas Garnier  wrote:
> On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  wrote:
>> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
>>> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>>>
 So I used your .config to generate one for my test machine and with
 that I can reproduce.
>>>
>>> Was that the config I've sent, or did Boris provide one as well? Which one
>>> are you able to reproduce with please?
>>
>> It's the Boris' one.
>>
>> Moreover, I have found the options that make the difference: unsetting
>> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
>> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
>> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
>>
>> Unbelievable, but that's what I'm seeing.
>
> Nice find!
>
>>
>> Now, that leads to a few questions:
>>
>> - How does lockdep change the picture so it matters for hibernation?
>> - Why is hibernation the only piece that's affected?
>> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
>>
>> Thomas, any ideas?
>
> No idea so far. I will investigate though.
>
> We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
> don't think it was related because it was on early boot and with
> certain e820 memory layout (and PUD randomization that I disabled on
> the previous patch test). The fix is on tip:
> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d

Well, I don't think this is related.

In the meantime, I went back to my original .config and verified that
setting CONFIG_DEBUG_LOCK_ALLOC in it caused hibernation to fail (with
CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied), so
this really matters somehow.

Besides, now that I have a reproducer, I can check various other
things and for example this change (sorry for broken whitespace):

Index: linux-pm/arch/x86/mm/kaslr.c
===
--- linux-pm.orig/arch/x86/mm/kaslr.c
+++ linux-pm/arch/x86/mm/kaslr.c
@@ -122,7 +122,7 @@ void __init kernel_randomize_memory(void
 prandom_bytes_state(_state, , sizeof(rand));
 entropy = (rand % (entropy + 1)) & PUD_MASK;
 vaddr += entropy;
-*kaslr_regions[i].base = vaddr;
+*kaslr_regions[i].base += PUD_SIZE;

 /*
  * Jump the region and add a minimum padding based on

makes hibernation work for me again in the above configuration.  To
me, this means that the $subject patch works as expected and the
problem really is related to the vaddr value being too big.

Thanks,
Rafael

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

[Thomas Garnier]

On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  wrote:
> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
>> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>>
>>> So I used your .config to generate one for my test machine and with
>>> that I can reproduce.
>>
>> Was that the config I've sent, or did Boris provide one as well? Which one
>> are you able to reproduce with please?
>
> It's the Boris' one.
>
> Moreover, I have found the options that make the difference: unsetting
> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
>
> Unbelievable, but that's what I'm seeing.

Nice find!

>
> Now, that leads to a few questions:
>
> - How does lockdep change the picture so it matters for hibernation?
> - Why is hibernation the only piece that's affected?
> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
>
> Thomas, any ideas?

No idea so far. I will investigate though.

We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
don't think it was related because it was on early boot and with
certain e820 memory layout (and PUD randomization that I disabled on
the previous patch test). The fix is on tip:
http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d

>
> Thanks,
> Rafael

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

[Thomas Garnier]

On Wed, Aug 10, 2016 at 5:35 PM, Rafael J. Wysocki  wrote:
> On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
>> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>>
>>> So I used your .config to generate one for my test machine and with
>>> that I can reproduce.
>>
>> Was that the config I've sent, or did Boris provide one as well? Which one
>> are you able to reproduce with please?
>
> It's the Boris' one.
>
> Moreover, I have found the options that make the difference: unsetting
> CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
> unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
> CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.
>
> Unbelievable, but that's what I'm seeing.

Nice find!

>
> Now, that leads to a few questions:
>
> - How does lockdep change the picture so it matters for hibernation?
> - Why is hibernation the only piece that's affected?
> - Why is RANDOMIZE_MEMORY necessary to make this breakage show up?
>
> Thomas, any ideas?

No idea so far. I will investigate though.

We had an unrelated issue with CONFIG_DEBUG_PAGEALLOC on early boot. I
don't think it was related because it was on early boot and with
certain e820 memory layout (and PUD randomization that I disabled on
the previous patch test). The fix is on tip:
http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fb754f958f8e46202c1efd7f66d5b3db1208117d

>
> Thanks,
> Rafael

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

[Rafael J. Wysocki]

On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>
>> So I used your .config to generate one for my test machine and with
>> that I can reproduce.
>
> Was that the config I've sent, or did Boris provide one as well? Which one
> are you able to reproduce with please?

It's the Boris' one.

Moreover, I have found the options that make the difference: unsetting
CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.

Unbelievable, but that's what I'm seeing.

Now, that leads to a few questions:

- How does lockdep change the picture so it matters for hibernation?
- Why is hibernation the only piece that's affected?
- Why is RANDOMIZE_MEMORY necessary to make this breakage show up?

Thomas, any ideas?

Thanks,
Rafael

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

[Rafael J. Wysocki]

On Wed, Aug 10, 2016 at 11:59 PM, Jiri Kosina  wrote:
> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>
>> So I used your .config to generate one for my test machine and with
>> that I can reproduce.
>
> Was that the config I've sent, or did Boris provide one as well? Which one
> are you able to reproduce with please?

It's the Boris' one.

Moreover, I have found the options that make the difference: unsetting
CONFIG_PROVE_LOCKING and CONFIG_DEBUG_LOCK_ALLOC (which also will
unset CONFIG_LOCKDEP AFAICS) in it makes hibernation work again with
CONFIG_RANDOMIZE_MEMORY set and with the $subject patch applied.

Unbelievable, but that's what I'm seeing.

Now, that leads to a few questions:

- How does lockdep change the picture so it matters for hibernation?
- Why is hibernation the only piece that's affected?
- Why is RANDOMIZE_MEMORY necessary to make this breakage show up?

Thomas, any ideas?

Thanks,
Rafael

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

[Jiri Kosina]

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

> So I used your .config to generate one for my test machine and with
> that I can reproduce.

Was that the config I've sent, or did Boris provide one as well? Which one 
are you able to reproduce with please?

> The hardware configuration doesn't matter, then, the issue is config-related.

How big is the diff between the two configs? Could you please share it?

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

[Rafael J. Wysocki]

On Wed, Aug 10, 2016 at 11:52 PM, Jiri Kosina  wrote:
> On Wed, 10 Aug 2016, Thomas Garnier wrote:
>
>> 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(_state, , 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(_regions[i]);
>> -   vaddr = round_up(vaddr + 1, PUD_SIZE);
>> +   vaddr = round_up(vaddr + 1, PGDIR_SIZE);
>> remain_entropy -= entropy;
>> }
>>  }
>
> I applied this on top of both fixes from Rafael from this thread; still no
> change in behavior, i.e. reboot immediately after reading the hibernation
> image.

Same for me on the box where I can reproduce the problem now.

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

[Jiri Kosina]

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

> So I used your .config to generate one for my test machine and with
> that I can reproduce.

Was that the config I've sent, or did Boris provide one as well? Which one 
are you able to reproduce with please?

> The hardware configuration doesn't matter, then, the issue is config-related.

How big is the diff between the two configs? Could you please share it?

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

[Rafael J. Wysocki]

On Wed, Aug 10, 2016 at 11:52 PM, Jiri Kosina  wrote:
> On Wed, 10 Aug 2016, Thomas Garnier wrote:
>
>> 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(_state, , 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(_regions[i]);
>> -   vaddr = round_up(vaddr + 1, PUD_SIZE);
>> +   vaddr = round_up(vaddr + 1, PGDIR_SIZE);
>> remain_entropy -= entropy;
>> }
>>  }
>
> I applied this on top of both fixes from Rafael from this thread; still no
> change in behavior, i.e. reboot immediately after reading the hibernation
> image.

Same for me on the box where I can reproduce the problem now.

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

[Jiri Kosina]

On Wed, 10 Aug 2016, Thomas Garnier wrote:

> 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(_state, , 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(_regions[i]);
> -   vaddr = round_up(vaddr + 1, PUD_SIZE);
> +   vaddr = round_up(vaddr + 1, PGDIR_SIZE);
> remain_entropy -= entropy;
> }
>  }

I applied this on top of both fixes from Rafael from this thread; still no 
change in behavior, i.e. reboot immediately after reading the hibernation 
image.

-- 
Jiri Kosina
SUSE Labs

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

[Jiri Kosina]

On Wed, 10 Aug 2016, Thomas Garnier wrote:

> 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(_state, , 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(_regions[i]);
> -   vaddr = round_up(vaddr + 1, PUD_SIZE);
> +   vaddr = round_up(vaddr + 1, PGDIR_SIZE);
> remain_entropy -= entropy;
> }
>  }

I applied this on top of both fixes from Rafael from this thread; still no 
change in behavior, i.e. reboot immediately after reading the hibernation 
image.

-- 
Jiri Kosina
SUSE Labs

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

[Rafael J. Wysocki]

On Wed, Aug 10, 2016 at 10:56 PM, Rafael J. Wysocki  wrote:
> On Wed, Aug 10, 2016 at 6:35 PM, Borislav Petkov  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 0x-0x0009d7ff] usable
>> BIOS-e820: [mem 0x0009d800-0x0009] reserved
>> BIOS-e820: [mem 0x000e-0x000f] reserved
>> BIOS-e820: [mem 0x0010-0x1fff] usable
>> BIOS-e820: [mem 0x2000-0x201f] reserved
>> BIOS-e820: [mem 0x2020-0x40003fff] usable
>> BIOS-e820: [mem 0x40004000-0x40004fff] reserved
>> BIOS-e820: [mem 0x40005000-0xcec2] usable
>> BIOS-e820: [mem 0xcec3-0xdae9efff] reserved
>> BIOS-e820: [mem 0xdae9f000-0xdaf9efff] ACPI NVS
>> BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
>> BIOS-e820: [mem 0xdafff000-0xdf9f] reserved
>> BIOS-e820: [mem 0xf800-0xfbff] reserved
>> BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
>> BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
>> BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
>> BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
>> BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
>> BIOS-e820: [mem 0xffc0-0x] reserved
>> BIOS-e820: [mem 0x0001-0x00021e5f] usable
>> BIOS-e820: [mem 0x00021e60-0x00021e7f] 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 0x-0x0fff] usable ==> reserved
>> e820: remove [mem 0x000a-0x000f] usable
>> e820: last_pfn = 0x21e600 max_arch_pfn = 0x4
>
> So far, I'm unable to reproduce the problem (with the $subject patch
> applied) on two different Intel-base machines with 4 Gig and 8 Gig of
> RAM.

So I used your .config to generate one for my test machine and with
that I can reproduce.

The hardware configuration doesn't matter, then, the issue is config-related.

Thanks,
Rafael

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

[Rafael J. Wysocki]

On Wed, Aug 10, 2016 at 10:56 PM, Rafael J. Wysocki  wrote:
> On Wed, Aug 10, 2016 at 6:35 PM, Borislav Petkov  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 0x-0x0009d7ff] usable
>> BIOS-e820: [mem 0x0009d800-0x0009] reserved
>> BIOS-e820: [mem 0x000e-0x000f] reserved
>> BIOS-e820: [mem 0x0010-0x1fff] usable
>> BIOS-e820: [mem 0x2000-0x201f] reserved
>> BIOS-e820: [mem 0x2020-0x40003fff] usable
>> BIOS-e820: [mem 0x40004000-0x40004fff] reserved
>> BIOS-e820: [mem 0x40005000-0xcec2] usable
>> BIOS-e820: [mem 0xcec3-0xdae9efff] reserved
>> BIOS-e820: [mem 0xdae9f000-0xdaf9efff] ACPI NVS
>> BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
>> BIOS-e820: [mem 0xdafff000-0xdf9f] reserved
>> BIOS-e820: [mem 0xf800-0xfbff] reserved
>> BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
>> BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
>> BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
>> BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
>> BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
>> BIOS-e820: [mem 0xffc0-0x] reserved
>> BIOS-e820: [mem 0x0001-0x00021e5f] usable
>> BIOS-e820: [mem 0x00021e60-0x00021e7f] 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 0x-0x0fff] usable ==> reserved
>> e820: remove [mem 0x000a-0x000f] usable
>> e820: last_pfn = 0x21e600 max_arch_pfn = 0x4
>
> So far, I'm unable to reproduce the problem (with the $subject patch
> applied) on two different Intel-base machines with 4 Gig and 8 Gig of
> RAM.

So I used your .config to generate one for my test machine and with
that I can reproduce.

The hardware configuration doesn't matter, then, the issue is config-related.

Thanks,
Rafael

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

[Rafael J. Wysocki]

On Wed, Aug 10, 2016 at 6:35 PM, Borislav Petkov  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 0x-0x0009d7ff] usable
> BIOS-e820: [mem 0x0009d800-0x0009] reserved
> BIOS-e820: [mem 0x000e-0x000f] reserved
> BIOS-e820: [mem 0x0010-0x1fff] usable
> BIOS-e820: [mem 0x2000-0x201f] reserved
> BIOS-e820: [mem 0x2020-0x40003fff] usable
> BIOS-e820: [mem 0x40004000-0x40004fff] reserved
> BIOS-e820: [mem 0x40005000-0xcec2] usable
> BIOS-e820: [mem 0xcec3-0xdae9efff] reserved
> BIOS-e820: [mem 0xdae9f000-0xdaf9efff] ACPI NVS
> BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
> BIOS-e820: [mem 0xdafff000-0xdf9f] reserved
> BIOS-e820: [mem 0xf800-0xfbff] reserved
> BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
> BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
> BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
> BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
> BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
> BIOS-e820: [mem 0xffc0-0x] reserved
> BIOS-e820: [mem 0x0001-0x00021e5f] usable
> BIOS-e820: [mem 0x00021e60-0x00021e7f] 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 0x-0x0fff] usable ==> reserved
> e820: remove [mem 0x000a-0x000f] usable
> e820: last_pfn = 0x21e600 max_arch_pfn = 0x4

So far, I'm unable to reproduce the problem (with the $subject patch
applied) on two different Intel-base machines with 4 Gig and 8 Gig of
RAM.

One thing that's clearly different on my machines is that they both
have usable memory at the end of the e820 map (and the one where Jiri
can reproduce the problem has reserved memory at the end of it, just
like yours).

Thomas, what about the e820 map on your machine?

I'm not sure why that would matter, though.

Thanks,
Rafael

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

[Rafael J. Wysocki]

On Wed, Aug 10, 2016 at 6:35 PM, Borislav Petkov  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 0x-0x0009d7ff] usable
> BIOS-e820: [mem 0x0009d800-0x0009] reserved
> BIOS-e820: [mem 0x000e-0x000f] reserved
> BIOS-e820: [mem 0x0010-0x1fff] usable
> BIOS-e820: [mem 0x2000-0x201f] reserved
> BIOS-e820: [mem 0x2020-0x40003fff] usable
> BIOS-e820: [mem 0x40004000-0x40004fff] reserved
> BIOS-e820: [mem 0x40005000-0xcec2] usable
> BIOS-e820: [mem 0xcec3-0xdae9efff] reserved
> BIOS-e820: [mem 0xdae9f000-0xdaf9efff] ACPI NVS
> BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
> BIOS-e820: [mem 0xdafff000-0xdf9f] reserved
> BIOS-e820: [mem 0xf800-0xfbff] reserved
> BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
> BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
> BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
> BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
> BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
> BIOS-e820: [mem 0xffc0-0x] reserved
> BIOS-e820: [mem 0x0001-0x00021e5f] usable
> BIOS-e820: [mem 0x00021e60-0x00021e7f] 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 0x-0x0fff] usable ==> reserved
> e820: remove [mem 0x000a-0x000f] usable
> e820: last_pfn = 0x21e600 max_arch_pfn = 0x4

So far, I'm unable to reproduce the problem (with the $subject patch
applied) on two different Intel-base machines with 4 Gig and 8 Gig of
RAM.

One thing that's clearly different on my machines is that they both
have usable memory at the end of the e820 map (and the one where Jiri
can reproduce the problem has reserved memory at the end of it, just
like yours).

Thomas, what about the e820 map on your machine?

I'm not sure why that would matter, though.

Thanks,
Rafael

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

[Jiri Kosina]

On Wed, 10 Aug 2016, Thomas Garnier wrote:

> What type of machines are you testing it on? What is the memory size? 
> Processor generation?

Mine is Lenovo thinkpad x200s; I think Boris has been testing it on x230s, 
but not sure whether any of the latest patches didn't actually fix it for 
him.

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

 BIOS-e820: [mem 0x-0x0009ebff] usable
 BIOS-e820: [mem 0x0009ec00-0x0009] reserved
 BIOS-e820: [mem 0x000dc000-0x000f] reserved
 BIOS-e820: [mem 0x0010-0x7c4a0fff] usable
 BIOS-e820: [mem 0x7c4a1000-0x7c4a6fff] reserved
 BIOS-e820: [mem 0x7c4a7000-0x7c5b6fff] usable
 BIOS-e820: [mem 0x7c5b7000-0x7c60efff] reserved
 BIOS-e820: [mem 0x7c60f000-0x7c6c5fff] usable
 BIOS-e820: [mem 0x7c6c6000-0x7c6d0fff] ACPI NVS
 BIOS-e820: [mem 0x7c6d1000-0x7c6d3fff] ACPI data
 BIOS-e820: [mem 0x7c6d4000-0x7c6d7fff] reserved
 BIOS-e820: [mem 0x7c6d8000-0x7c6dbfff] ACPI NVS
 BIOS-e820: [mem 0x7c6dc000-0x7c6defff] reserved
 BIOS-e820: [mem 0x7c6df000-0x7c705fff] ACPI NVS
 BIOS-e820: [mem 0x7c706000-0x7c707fff] ACPI data
 BIOS-e820: [mem 0x7c708000-0x7c90efff] reserved
 BIOS-e820: [mem 0x7c90f000-0x7c99efff] ACPI NVS
 BIOS-e820: [mem 0x7c99f000-0x7c9fefff] ACPI data
 BIOS-e820: [mem 0x7c9ff000-0x7c9f] usable
 BIOS-e820: [mem 0x7cc0-0x7eff] reserved
 BIOS-e820: [mem 0xe000-0xefff] reserved
 BIOS-e820: [mem 0xfec0-0xfec0] reserved
 BIOS-e820: [mem 0xfed0-0xfed003ff] reserved
 BIOS-e820: [mem 0xfed1-0xfed13fff] reserved
 BIOS-e820: [mem 0xfed18000-0xfed19fff] reserved
 BIOS-e820: [mem 0xfed1c000-0xfed8] reserved
 BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
 BIOS-e820: [mem 0xff80-0x] reserved
 NX (Execute Disable) protection: active
 SMBIOS 2.4 present.
 DMI: LENOVO 7470BN2/7470BN2, BIOS 6DET38WW (2.02 ) 12/19/2008
 e820: update [mem 0x-0x0fff] usable ==> reserved
 e820: remove [mem 0x000a-0x000f] usable
 e820: last_pfn = 0x7ca00 max_arch_pfn = 0x4


CPU:

processor   : 0
vendor_id   : GenuineIntel
cpu family  : 6
model   : 23
model name  : Intel(R) Core(TM)2 Duo CPU L9400  @ 1.86GHz
stepping: 6
microcode   : 0x60c
cpu MHz : 800.000
cache size  : 6144 KB
physical id : 0
siblings: 2
core id : 0
cpu cores   : 2
apicid  : 0
initial apicid  : 0
fpu : yes
fpu_exception   : yes
cpuid level : 10
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 lm 
constant_tsc arch_perfmon pebs bts nopl aperfmperf eagerfpu pni dtes64 monitor 
ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm sse4_1 lahf_lm tpr_shadow vnmi 
flexpriority dtherm ida
bugs:
bogomips: 3723.69
clflush size: 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
power management:

-- 
Jiri Kosina
SUSE Labs

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

[Jiri Kosina]

On Wed, 10 Aug 2016, Thomas Garnier wrote:

> What type of machines are you testing it on? What is the memory size? 
> Processor generation?

Mine is Lenovo thinkpad x200s; I think Boris has been testing it on x230s, 
but not sure whether any of the latest patches didn't actually fix it for 
him.

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

 BIOS-e820: [mem 0x-0x0009ebff] usable
 BIOS-e820: [mem 0x0009ec00-0x0009] reserved
 BIOS-e820: [mem 0x000dc000-0x000f] reserved
 BIOS-e820: [mem 0x0010-0x7c4a0fff] usable
 BIOS-e820: [mem 0x7c4a1000-0x7c4a6fff] reserved
 BIOS-e820: [mem 0x7c4a7000-0x7c5b6fff] usable
 BIOS-e820: [mem 0x7c5b7000-0x7c60efff] reserved
 BIOS-e820: [mem 0x7c60f000-0x7c6c5fff] usable
 BIOS-e820: [mem 0x7c6c6000-0x7c6d0fff] ACPI NVS
 BIOS-e820: [mem 0x7c6d1000-0x7c6d3fff] ACPI data
 BIOS-e820: [mem 0x7c6d4000-0x7c6d7fff] reserved
 BIOS-e820: [mem 0x7c6d8000-0x7c6dbfff] ACPI NVS
 BIOS-e820: [mem 0x7c6dc000-0x7c6defff] reserved
 BIOS-e820: [mem 0x7c6df000-0x7c705fff] ACPI NVS
 BIOS-e820: [mem 0x7c706000-0x7c707fff] ACPI data
 BIOS-e820: [mem 0x7c708000-0x7c90efff] reserved
 BIOS-e820: [mem 0x7c90f000-0x7c99efff] ACPI NVS
 BIOS-e820: [mem 0x7c99f000-0x7c9fefff] ACPI data
 BIOS-e820: [mem 0x7c9ff000-0x7c9f] usable
 BIOS-e820: [mem 0x7cc0-0x7eff] reserved
 BIOS-e820: [mem 0xe000-0xefff] reserved
 BIOS-e820: [mem 0xfec0-0xfec0] reserved
 BIOS-e820: [mem 0xfed0-0xfed003ff] reserved
 BIOS-e820: [mem 0xfed1-0xfed13fff] reserved
 BIOS-e820: [mem 0xfed18000-0xfed19fff] reserved
 BIOS-e820: [mem 0xfed1c000-0xfed8] reserved
 BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
 BIOS-e820: [mem 0xff80-0x] reserved
 NX (Execute Disable) protection: active
 SMBIOS 2.4 present.
 DMI: LENOVO 7470BN2/7470BN2, BIOS 6DET38WW (2.02 ) 12/19/2008
 e820: update [mem 0x-0x0fff] usable ==> reserved
 e820: remove [mem 0x000a-0x000f] usable
 e820: last_pfn = 0x7ca00 max_arch_pfn = 0x4


CPU:

processor   : 0
vendor_id   : GenuineIntel
cpu family  : 6
model   : 23
model name  : Intel(R) Core(TM)2 Duo CPU L9400  @ 1.86GHz
stepping: 6
microcode   : 0x60c
cpu MHz : 800.000
cache size  : 6144 KB
physical id : 0
siblings: 2
core id : 0
cpu cores   : 2
apicid  : 0
initial apicid  : 0
fpu : yes
fpu_exception   : yes
cpuid level : 10
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 lm 
constant_tsc arch_perfmon pebs bts nopl aperfmperf eagerfpu pni dtes64 monitor 
ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm sse4_1 lahf_lm tpr_shadow vnmi 
flexpriority dtherm ida
bugs:
bogomips: 3723.69
clflush size: 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
power management:

-- 
Jiri Kosina
SUSE Labs

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

[Rafael J. Wysocki]

On Wednesday, August 10, 2016 09:50:15 AM Jiri Kosina wrote:
> 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.

The last patch I sent had a problem, because if restore_jump_address really
overlapped with the identity mapping of the restore kernel, it might share
PGD or PUD entries with that mapping and that should have been taken into
account.

Here goes an update.  Again, this works on my test machine, but then the
previous version worked on it too ...

---
 arch/x86/power/hibernate_64.c |   53 ++
 arch/x86/power/hibernate_asm_64.S |   10 +++
 2 files changed, 53 insertions(+), 10 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,22 @@ 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)
 {
+   unsigned long pgd_idx = pgd_index(restore_jump_address);
+   unsigned long pud_idx = pud_index(restore_jump_address);
+   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
@@ -69,10 +77,32 @@ static int set_up_temporary_text_mapping
 
set_pmd(pmd + pmd_index(restore_jump_address),
__pmd((jump_address_phys & PMD_MASK) | 
__PAGE_KERNEL_LARGE_EXEC));
-   set_pud(pud + pud_index(restore_jump_address),
+   set_pud(pud + pud_idx, __pud(__pa(pmd) | _KERNPG_TABLE));
+   set_pgd(pgd + pgd_idx, __pgd(__pa(pud) | _KERNPG_TABLE));
+
+   if (pgd_idx != pgd_index(relocated_restore_code)) {
+   pud = (pud_t *)get_safe_page(GFP_ATOMIC);
+   if (!pud)
+   return -ENOMEM;
+
+   set_pgd(pgd + pgd_index(relocated_restore_code),
+   __pgd(__pa(pud) | _KERNPG_TABLE));
+   } else if (pud_idx == pud_index(relocated_restore_code)) {
+   goto set_pmd;
+   }
+
+   pmd = (pmd_t *)get_safe_page(GFP_ATOMIC);
+   if (!pmd)
+   return -ENOMEM;
+
+   set_pud(pud + pud_index(relocated_restore_code),
__pud(__pa(pmd) | _KERNPG_TABLE));
-   set_pgd(pgd + pgd_index(restore_jump_address),
-   __pgd(__pa(pud) | _KERNPG_TABLE));
+
+ set_pmd:
+   set_pmd(pmd + pmd_index(relocated_restore_code),
+   __pmd((__pa(relocated_restore_code) & PMD_MASK) | 
__PAGE_KERNEL_LARGE_EXEC));
+
+   jump_level4_pgt = __pa(pgd);
 
return 0;
 }
@@ -98,11 +128,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 +147,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 +190,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

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

[Rafael J. Wysocki]

On Wednesday, August 10, 2016 09:50:15 AM Jiri Kosina wrote:
> 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.

The last patch I sent had a problem, because if restore_jump_address really
overlapped with the identity mapping of the restore kernel, it might share
PGD or PUD entries with that mapping and that should have been taken into
account.

Here goes an update.  Again, this works on my test machine, but then the
previous version worked on it too ...

---
 arch/x86/power/hibernate_64.c |   53 ++
 arch/x86/power/hibernate_asm_64.S |   10 +++
 2 files changed, 53 insertions(+), 10 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,22 @@ 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)
 {
+   unsigned long pgd_idx = pgd_index(restore_jump_address);
+   unsigned long pud_idx = pud_index(restore_jump_address);
+   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
@@ -69,10 +77,32 @@ static int set_up_temporary_text_mapping
 
set_pmd(pmd + pmd_index(restore_jump_address),
__pmd((jump_address_phys & PMD_MASK) | 
__PAGE_KERNEL_LARGE_EXEC));
-   set_pud(pud + pud_index(restore_jump_address),
+   set_pud(pud + pud_idx, __pud(__pa(pmd) | _KERNPG_TABLE));
+   set_pgd(pgd + pgd_idx, __pgd(__pa(pud) | _KERNPG_TABLE));
+
+   if (pgd_idx != pgd_index(relocated_restore_code)) {
+   pud = (pud_t *)get_safe_page(GFP_ATOMIC);
+   if (!pud)
+   return -ENOMEM;
+
+   set_pgd(pgd + pgd_index(relocated_restore_code),
+   __pgd(__pa(pud) | _KERNPG_TABLE));
+   } else if (pud_idx == pud_index(relocated_restore_code)) {
+   goto set_pmd;
+   }
+
+   pmd = (pmd_t *)get_safe_page(GFP_ATOMIC);
+   if (!pmd)
+   return -ENOMEM;
+
+   set_pud(pud + pud_index(relocated_restore_code),
__pud(__pa(pmd) | _KERNPG_TABLE));
-   set_pgd(pgd + pgd_index(restore_jump_address),
-   __pgd(__pa(pud) | _KERNPG_TABLE));
+
+ set_pmd:
+   set_pmd(pmd + pmd_index(relocated_restore_code),
+   __pmd((__pa(relocated_restore_code) & PMD_MASK) | 
__PAGE_KERNEL_LARGE_EXEC));
+
+   jump_level4_pgt = __pa(pgd);
 
return 0;
 }
@@ -98,11 +128,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 +147,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 +190,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

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

[Thomas Garnier]

On Wed, Aug 10, 2016 at 6:18 AM, Jiri Kosina  wrote:
> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>
>> The last patch I sent had a problem, because if restore_jump_address really
>> overlapped with the identity mapping of the restore kernel, it might share
>> PGD or PUD entries with that mapping and that should have been taken into
>> account.
>>
>> Here goes an update.  Again, this works on my test machine, but then the
>> previous version worked on it too ...
>
> Unfortunately still exactly the same symptoms during resume even with this
> one.

What type of machines are you testing it on? What is the memory size?
Processor generation?

>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

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

[Thomas Garnier]

On Wed, Aug 10, 2016 at 6:18 AM, Jiri Kosina  wrote:
> On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
>
>> The last patch I sent had a problem, because if restore_jump_address really
>> overlapped with the identity mapping of the restore kernel, it might share
>> PGD or PUD entries with that mapping and that should have been taken into
>> account.
>>
>> Here goes an update.  Again, this works on my test machine, but then the
>> previous version worked on it too ...
>
> Unfortunately still exactly the same symptoms during resume even with this
> one.

What type of machines are you testing it on? What is the memory size?
Processor generation?

>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

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

[Thomas Garnier]

On Wed, Aug 10, 2016 at 9:35 AM, Borislav Petkov  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 0x-0x0009d7ff] usable
> BIOS-e820: [mem 0x0009d800-0x0009] reserved
> BIOS-e820: [mem 0x000e-0x000f] reserved
> BIOS-e820: [mem 0x0010-0x1fff] usable
> BIOS-e820: [mem 0x2000-0x201f] reserved
> BIOS-e820: [mem 0x2020-0x40003fff] usable
> BIOS-e820: [mem 0x40004000-0x40004fff] reserved
> BIOS-e820: [mem 0x40005000-0xcec2] usable
> BIOS-e820: [mem 0xcec3-0xdae9efff] reserved
> BIOS-e820: [mem 0xdae9f000-0xdaf9efff] ACPI NVS
> BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
> BIOS-e820: [mem 0xdafff000-0xdf9f] reserved
> BIOS-e820: [mem 0xf800-0xfbff] reserved
> BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
> BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
> BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
> BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
> BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
> BIOS-e820: [mem 0xffc0-0x] reserved
> BIOS-e820: [mem 0x0001-0x00021e5f] usable
> BIOS-e820: [mem 0x00021e60-0x00021e7f] 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 0x-0x0fff] usable ==> reserved
> e820: remove [mem 0x000a-0x000f] usable
> e820: last_pfn = 0x21e600 max_arch_pfn = 0x4
>
>> 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(_state, , 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(_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)
> --

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

[Thomas Garnier]

On Wed, Aug 10, 2016 at 9:35 AM, Borislav Petkov  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 0x-0x0009d7ff] usable
> BIOS-e820: [mem 0x0009d800-0x0009] reserved
> BIOS-e820: [mem 0x000e-0x000f] reserved
> BIOS-e820: [mem 0x0010-0x1fff] usable
> BIOS-e820: [mem 0x2000-0x201f] reserved
> BIOS-e820: [mem 0x2020-0x40003fff] usable
> BIOS-e820: [mem 0x40004000-0x40004fff] reserved
> BIOS-e820: [mem 0x40005000-0xcec2] usable
> BIOS-e820: [mem 0xcec3-0xdae9efff] reserved
> BIOS-e820: [mem 0xdae9f000-0xdaf9efff] ACPI NVS
> BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
> BIOS-e820: [mem 0xdafff000-0xdf9f] reserved
> BIOS-e820: [mem 0xf800-0xfbff] reserved
> BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
> BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
> BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
> BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
> BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
> BIOS-e820: [mem 0xffc0-0x] reserved
> BIOS-e820: [mem 0x0001-0x00021e5f] usable
> BIOS-e820: [mem 0x00021e60-0x00021e7f] 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 0x-0x0fff] usable ==> reserved
> e820: remove [mem 0x000a-0x000f] usable
> e820: last_pfn = 0x21e600 max_arch_pfn = 0x4
>
>> 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(_state, , 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(_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)
> --

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

[Borislav Petkov]

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 0x-0x0009d7ff] usable
BIOS-e820: [mem 0x0009d800-0x0009] reserved
BIOS-e820: [mem 0x000e-0x000f] reserved
BIOS-e820: [mem 0x0010-0x1fff] usable
BIOS-e820: [mem 0x2000-0x201f] reserved
BIOS-e820: [mem 0x2020-0x40003fff] usable
BIOS-e820: [mem 0x40004000-0x40004fff] reserved
BIOS-e820: [mem 0x40005000-0xcec2] usable
BIOS-e820: [mem 0xcec3-0xdae9efff] reserved
BIOS-e820: [mem 0xdae9f000-0xdaf9efff] ACPI NVS
BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
BIOS-e820: [mem 0xdafff000-0xdf9f] reserved
BIOS-e820: [mem 0xf800-0xfbff] reserved
BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
BIOS-e820: [mem 0xffc0-0x] reserved
BIOS-e820: [mem 0x0001-0x00021e5f] usable
BIOS-e820: [mem 0x00021e60-0x00021e7f] 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 0x-0x0fff] usable ==> reserved
e820: remove [mem 0x000a-0x000f] usable
e820: last_pfn = 0x21e600 max_arch_pfn = 0x4

> 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)
--

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

[Jiri Kosina]

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

> The last patch I sent had a problem, because if restore_jump_address really
> overlapped with the identity mapping of the restore kernel, it might share
> PGD or PUD entries with that mapping and that should have been taken into
> account.
> 
> Here goes an update.  Again, this works on my test machine, but then the
> previous version worked on it too ...

Unfortunately still exactly the same symptoms during resume even with this 
one.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

[Jiri Kosina]

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

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

[Borislav Petkov]

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 0x-0x0009d7ff] usable
BIOS-e820: [mem 0x0009d800-0x0009] reserved
BIOS-e820: [mem 0x000e-0x000f] reserved
BIOS-e820: [mem 0x0010-0x1fff] usable
BIOS-e820: [mem 0x2000-0x201f] reserved
BIOS-e820: [mem 0x2020-0x40003fff] usable
BIOS-e820: [mem 0x40004000-0x40004fff] reserved
BIOS-e820: [mem 0x40005000-0xcec2] usable
BIOS-e820: [mem 0xcec3-0xdae9efff] reserved
BIOS-e820: [mem 0xdae9f000-0xdaf9efff] ACPI NVS
BIOS-e820: [mem 0xdaf9f000-0xdaffefff] ACPI data
BIOS-e820: [mem 0xdafff000-0xdf9f] reserved
BIOS-e820: [mem 0xf800-0xfbff] reserved
BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
BIOS-e820: [mem 0xfed08000-0xfed08fff] reserved
BIOS-e820: [mem 0xfed1-0xfed19fff] reserved
BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
BIOS-e820: [mem 0xffc0-0x] reserved
BIOS-e820: [mem 0x0001-0x00021e5f] usable
BIOS-e820: [mem 0x00021e60-0x00021e7f] 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 0x-0x0fff] usable ==> reserved
e820: remove [mem 0x000a-0x000f] usable
e820: last_pfn = 0x21e600 max_arch_pfn = 0x4

> 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)
--

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

[Jiri Kosina]

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

> The last patch I sent had a problem, because if restore_jump_address really
> overlapped with the identity mapping of the restore kernel, it might share
> PGD or PUD entries with that mapping and that should have been taken into
> account.
> 
> Here goes an update.  Again, this works on my test machine, but then the
> previous version worked on it too ...

Unfortunately still exactly the same symptoms during resume even with this 
one.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

[Jiri Kosina]

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

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

[Rafael J. Wysocki]

On Tuesday, August 09, 2016 11:23:31 PM Rafael J. Wysocki wrote:
> On Tue, Aug 9, 2016 at 10:02 PM, Jiri Kosina  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

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

[Rafael J. Wysocki]

On Tuesday, August 09, 2016 11:23:31 PM Rafael J. Wysocki wrote:
> On Tue, Aug 9, 2016 at 10:02 PM, Jiri Kosina  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
===

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 10:02 PM, Jiri Kosina  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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 10:02 PM, Jiri Kosina  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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 6:27 PM, Thomas Garnier  wrote:
> On Tue, Aug 9, 2016 at 9:18 AM, Rafael J. Wysocki  wrote:
>> On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina  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 
 >>   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=0x100
 CONFIG_RELOCATABLE=y
 CONFIG_RANDOMIZE_BASE=y
 CONFIG_X86_NEED_RELOCS=y
 CONFIG_PHYSICAL_ALIGN=0x100
 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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 6:27 PM, Thomas Garnier  wrote:
> On Tue, Aug 9, 2016 at 9:18 AM, Rafael J. Wysocki  wrote:
>> On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina  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 
 >>   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=0x100
 CONFIG_RELOCATABLE=y
 CONFIG_RANDOMIZE_BASE=y
 CONFIG_X86_NEED_RELOCS=y
 CONFIG_PHYSICAL_ALIGN=0x100
 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

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

[Jiri Kosina]

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

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

[Jiri Kosina]

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

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

[Thomas Garnier]

On Tue, Aug 9, 2016 at 9:18 AM, Rafael J. Wysocki  wrote:
> On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina  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 
>>> >>   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=0x100
>>> CONFIG_RELOCATABLE=y
>>> CONFIG_RANDOMIZE_BASE=y
>>> CONFIG_X86_NEED_RELOCS=y
>>> CONFIG_PHYSICAL_ALIGN=0x100
>>> 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

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

[Thomas Garnier]

On Tue, Aug 9, 2016 at 9:18 AM, Rafael J. Wysocki  wrote:
> On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina  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 
>>> >>   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=0x100
>>> CONFIG_RELOCATABLE=y
>>> CONFIG_RANDOMIZE_BASE=y
>>> CONFIG_X86_NEED_RELOCS=y
>>> CONFIG_PHYSICAL_ALIGN=0x100
>>> 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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina  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 
>> >>   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=0x100
>> CONFIG_RELOCATABLE=y
>> CONFIG_RANDOMIZE_BASE=y
>> CONFIG_X86_NEED_RELOCS=y
>> CONFIG_PHYSICAL_ALIGN=0x100
>> 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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 5:05 PM, Jiri Kosina  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 
>> >>   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=0x100
>> CONFIG_RELOCATABLE=y
>> CONFIG_RANDOMIZE_BASE=y
>> CONFIG_X86_NEED_RELOCS=y
>> CONFIG_PHYSICAL_ALIGN=0x100
>> 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

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

[Jiri Kosina]

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 
> >>   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=0x100
> CONFIG_RELOCATABLE=y
> CONFIG_RANDOMIZE_BASE=y
> CONFIG_X86_NEED_RELOCS=y
> CONFIG_PHYSICAL_ALIGN=0x100
> 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

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

[Jiri Kosina]

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 
> >>   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=0x100
> CONFIG_RELOCATABLE=y
> CONFIG_RANDOMIZE_BASE=y
> CONFIG_X86_NEED_RELOCS=y
> CONFIG_PHYSICAL_ALIGN=0x100
> 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

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

[Jiri Kosina]

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 
>   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

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

[Jiri Kosina]

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 
>   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

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

[Jiri Kosina]

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 
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

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

[Jiri Kosina]

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 
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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 11:23 AM, Jiri Kosina  wrote:
> On Mon, 8 Aug 2016, Rafael J. Wysocki wrote:
>
>> From: Rafael J. Wysocki 
>>
>> 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 
>> Suggested-by: Yinghai Lu 
>> Signed-off-by: Rafael J. Wysocki 
>> Acked-by: Yinghai Lu 
>> ---
>>
>> 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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 11:23 AM, Jiri Kosina  wrote:
> On Mon, 8 Aug 2016, Rafael J. Wysocki wrote:
>
>> From: Rafael J. Wysocki 
>>
>> 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 
>> Suggested-by: Yinghai Lu 
>> Signed-off-by: Rafael J. Wysocki 
>> Acked-by: Yinghai Lu 
>> ---
>>
>> 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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 9:02 AM, Borislav Petkov  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

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

[Rafael J. Wysocki]

On Tue, Aug 9, 2016 at 9:02 AM, Borislav Petkov  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

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

[Jiri Kosina]

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

> From: Rafael J. Wysocki 
> 
> 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 
> Suggested-by: Yinghai Lu 
> Signed-off-by: Rafael J. Wysocki 
> Acked-by: Yinghai Lu 
> ---
> 
> 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

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

[Jiri Kosina]

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

> From: Rafael J. Wysocki 
> 
> 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 
> Suggested-by: Yinghai Lu 
> Signed-off-by: Rafael J. Wysocki 
> Acked-by: Yinghai Lu 
> ---
> 
> 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

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

[Borislav Petkov]

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.

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)
--
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.8.0-rc1 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=28
CONFIG_ARCH_MMAP_RND_BITS_MAX=32
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_DEBUG_RODATA=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="nazgul"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
CONFIG_AUDIT=y
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_GENERIC_MSI_IRQ_DOMAIN=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set
# CONFIG_IRQ_TIME_ACCOUNTING is not set
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y

#
# RCU Subsystem
#
CONFIG_PREEMPT_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
# CONFIG_TASKS_RCU is not set
CONFIG_RCU_STALL_COMMON=y
# CONFIG_TREE_RCU_TRACE is not set
# CONFIG_RCU_EXPEDITE_BOOT is not set
CONFIG_BUILD_BIN2C=y
CONFIG_IKCONFIG=m
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=21
CONFIG_LOG_CPU_MAX_BUF_SHIFT=12
CONFIG_NMI_LOG_BUF_SHIFT=13
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH=y
CONFIG_ARCH_SUPPORTS_INT128=y
CONFIG_NUMA_BALANCING=y
CONFIG_NUMA_BALANCING_DEFAULT_ENABLED=y
CONFIG_CGROUPS=y
# CONFIG_MEMCG is not set
# CONFIG_BLK_CGROUP is not set
CONFIG_CGROUP_SCHED=y
CONFIG_FAIR_GROUP_SCHED=y
# CONFIG_CFS_BANDWIDTH is not set
# CONFIG_RT_GROUP_SCHED is not set
# CONFIG_CGROUP_PIDS is not set
# CONFIG_CGROUP_FREEZER is not set
# CONFIG_CGROUP_HUGETLB is not set
# CONFIG_CPUSETS is not set
# CONFIG_CGROUP_DEVICE is not set
# CONFIG_CGROUP_CPUACCT is not set
# CONFIG_CGROUP_PERF is not set
# CONFIG_CGROUP_DEBUG is not set
# CONFIG_CHECKPOINT_RESTORE is not set
CONFIG_NAMESPACES=y
# 

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

[Borislav Petkov]

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.

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)
--
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.8.0-rc1 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=28
CONFIG_ARCH_MMAP_RND_BITS_MAX=32
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_DEBUG_RODATA=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="nazgul"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
CONFIG_AUDIT=y
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_GENERIC_MSI_IRQ_DOMAIN=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set
# CONFIG_IRQ_TIME_ACCOUNTING is not set
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
CONFIG_TASKSTATS=y
CONFIG_TASK_DELAY_ACCT=y
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y

#
# RCU Subsystem
#
CONFIG_PREEMPT_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
# CONFIG_TASKS_RCU is not set
CONFIG_RCU_STALL_COMMON=y
# CONFIG_TREE_RCU_TRACE is not set
# CONFIG_RCU_EXPEDITE_BOOT is not set
CONFIG_BUILD_BIN2C=y
CONFIG_IKCONFIG=m
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=21
CONFIG_LOG_CPU_MAX_BUF_SHIFT=12
CONFIG_NMI_LOG_BUF_SHIFT=13
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH=y
CONFIG_ARCH_SUPPORTS_INT128=y
CONFIG_NUMA_BALANCING=y
CONFIG_NUMA_BALANCING_DEFAULT_ENABLED=y
CONFIG_CGROUPS=y
# CONFIG_MEMCG is not set
# CONFIG_BLK_CGROUP is not set
CONFIG_CGROUP_SCHED=y
CONFIG_FAIR_GROUP_SCHED=y
# CONFIG_CFS_BANDWIDTH is not set
# CONFIG_RT_GROUP_SCHED is not set
# CONFIG_CGROUP_PIDS is not set
# CONFIG_CGROUP_FREEZER is not set
# CONFIG_CGROUP_HUGETLB is not set
# CONFIG_CPUSETS is not set
# CONFIG_CGROUP_DEVICE is not set
# CONFIG_CGROUP_CPUACCT is not set
# CONFIG_CGROUP_PERF is not set
# CONFIG_CGROUP_DEBUG is not set
# CONFIG_CHECKPOINT_RESTORE is not set
CONFIG_NAMESPACES=y
# 

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

[Rafael J. Wysocki]

On Mon, Aug 8, 2016 at 8:00 PM, Thomas Garnier  wrote:
> On Mon, Aug 8, 2016 at 6:54 AM, Rafael J. Wysocki  wrote:
>> On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov  wrote:
>>> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki 

 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 
>>>
>>> Reported-by: Borislav Petkov 
>>>
 Suggested-by: Yinghai Lu 
 Signed-off-by: Rafael J. Wysocki 
 Acked-by: Yinghai Lu 
 ---

 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!

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

[Rafael J. Wysocki]

On Mon, Aug 8, 2016 at 8:00 PM, Thomas Garnier  wrote:
> On Mon, Aug 8, 2016 at 6:54 AM, Rafael J. Wysocki  wrote:
>> On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov  wrote:
>>> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki 

 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 
>>>
>>> Reported-by: Borislav Petkov 
>>>
 Suggested-by: Yinghai Lu 
 Signed-off-by: Rafael J. Wysocki 
 Acked-by: Yinghai Lu 
 ---

 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!

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

[Thomas Garnier]

On Mon, Aug 8, 2016 at 6:54 AM, Rafael J. Wysocki  wrote:
> On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov  wrote:
>> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki 
>>>
>>> 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 
>>
>> Reported-by: Borislav Petkov 
>>
>>> Suggested-by: Yinghai Lu 
>>> Signed-off-by: Rafael J. Wysocki 
>>> Acked-by: Yinghai Lu 
>>> ---
>>>
>>> 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

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

[Thomas Garnier]

On Mon, Aug 8, 2016 at 6:54 AM, Rafael J. Wysocki  wrote:
> On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov  wrote:
>> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki 
>>>
>>> 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 
>>
>> Reported-by: Borislav Petkov 
>>
>>> Suggested-by: Yinghai Lu 
>>> Signed-off-by: Rafael J. Wysocki 
>>> Acked-by: Yinghai Lu 
>>> ---
>>>
>>> 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

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

[Rafael J. Wysocki]

On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov  wrote:
> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki 
>>
>> 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 
>
> Reported-by: Borislav Petkov 
>
>> Suggested-by: Yinghai Lu 
>> Signed-off-by: Rafael J. Wysocki 
>> Acked-by: Yinghai Lu 
>> ---
>>
>> 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

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

[Rafael J. Wysocki]

On Mon, Aug 8, 2016 at 3:40 PM, Borislav Petkov  wrote:
> On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki 
>>
>> 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 
>
> Reported-by: Borislav Petkov 
>
>> Suggested-by: Yinghai Lu 
>> Signed-off-by: Rafael J. Wysocki 
>> Acked-by: Yinghai Lu 
>> ---
>>
>> 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

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

[Borislav Petkov]

On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> 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 

Reported-by: Borislav Petkov 

> Suggested-by: Yinghai Lu 
> Signed-off-by: Rafael J. Wysocki 
> Acked-by: Yinghai Lu 
> ---
> 
> 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)
--

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

[Borislav Petkov]

On Mon, Aug 08, 2016 at 03:31:31PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> 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 

Reported-by: Borislav Petkov 

> Suggested-by: Yinghai Lu 
> Signed-off-by: Rafael J. Wysocki 
> Acked-by: Yinghai Lu 
> ---
> 
> 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)
--

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

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

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 
Suggested-by: Yinghai Lu 
Signed-off-by: Rafael J. Wysocki 
Acked-by: Yinghai Lu 
---

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

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

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

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 
Suggested-by: Yinghai Lu 
Signed-off-by: Rafael J. Wysocki 
Acked-by: Yinghai Lu 
---

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   =