[BISECTED] kexec regression on PowerBook G4

[BISECTED] kexec regression on PowerBook G4

[Aaro Koskinen]

Hi,

I was trying to upgrade from v5.0 -> v5.1 on PowerBook G4, but when trying
to kexec a kernel the system gets stuck (no errors seen on the console).

Bisected to: 93c4a162b014 ("powerpc/6xx: Store PGDIR physical address
in a SPRG"). This commit doesn't revert cleanly anymore but I tested
that the one before works OK.

With current Linus HEAD (9c7db5004280), it gets a bit further but still
doesn't work: now I get an error on the console after kexec "Starting
new kernel! ... Bye!":

	kernel tried to execute exec-protected page (...) - exploit attempt?

A.

Re: [BISECTED] kexec regression on PowerBook G4

[Christophe Leroy]

Hi Aero,

Le 22/05/2019 à 00:18, Aaro Koskinen a écrit :
> Hi,
> 
> I was trying to upgrade from v5.0 -> v5.1 on PowerBook G4, but when trying
> to kexec a kernel the system gets stuck (no errors seen on the console).

Do you mean you are trying to kexec a v5.1 kernel from a v5.0 kernel, or 
do you have a working v5.1 kernel, but kexec doesn't work with it ?

> 
> Bisected to: 93c4a162b014 ("powerpc/6xx: Store PGDIR physical address
> in a SPRG"). This commit doesn't revert cleanly anymore but I tested
> that the one before works OK.

Not sure that's the problem. There was a problem with that commit, but 
it was fixed by 4622a2d43101 ("powerpc/6xx: fix setup and use of 
SPRN_SPRG_PGDIR for hash32").
You probably hit some commit between those two during bisect, that's 
likely the reason why you ended here.

Can you restart your bisect from 4622a2d43101 ?

If you have CONFIG_SMP, maybe you should also consider taking 
397d2300b08c ("powerpc/32s: fix flush_hash_pages() on SMP"). Stable 
5.1.4 includes it.

> 
> With current Linus HEAD (9c7db5004280), it gets a bit further but still
> doesn't work: now I get an error on the console after kexec "Starting
> new kernel! ... Bye!":
> 
> 	kernel tried to execute exec-protected page (...) - exploit attempt?

Interesting.

Do you have CONFIG_STRICT_KERNEL_RWX=y in your .config ? If so, can you 
retry without it ?

Thanks
Christophe

> 
> A.
> 

Re: [BISECTED] kexec regression on PowerBook G4

[Christophe Leroy]

Hi Again,

On 05/22/2019 06:14 AM, Christophe Leroy wrote:
> Hi Aero,
> 
> Le 22/05/2019 à 00:18, Aaro Koskinen a écrit :
>> Hi,
>>
>> I was trying to upgrade from v5.0 -> v5.1 on PowerBook G4, but when 
>> trying
>> to kexec a kernel the system gets stuck (no errors seen on the console).
> 
> Do you mean you are trying to kexec a v5.1 kernel from a v5.0 kernel, or 
> do you have a working v5.1 kernel, but kexec doesn't work with it ?
> 
>>
>> Bisected to: 93c4a162b014 ("powerpc/6xx: Store PGDIR physical address
>> in a SPRG"). This commit doesn't revert cleanly anymore but I tested
>> that the one before works OK.
> 
> Not sure that's the problem. There was a problem with that commit, but 
> it was fixed by 4622a2d43101 ("powerpc/6xx: fix setup and use of 
> SPRN_SPRG_PGDIR for hash32").
> You probably hit some commit between those two during bisect, that's 
> likely the reason why you ended here.
> 
> Can you restart your bisect from 4622a2d43101 ?
> 
> If you have CONFIG_SMP, maybe you should also consider taking 
> 397d2300b08c ("powerpc/32s: fix flush_hash_pages() on SMP"). Stable 
> 5.1.4 includes it.
> 
>>
>> With current Linus HEAD (9c7db5004280), it gets a bit further but still
>> doesn't work: now I get an error on the console after kexec "Starting
>> new kernel! ... Bye!":
>>
>>     kernel tried to execute exec-protected page (...) - exploit attempt?
> 
> Interesting.
> 
> Do you have CONFIG_STRICT_KERNEL_RWX=y in your .config ? If so, can you 
> retry without it ?

After looking at the code, I don't thing CONFIG_STRICT_KERNEL_RWX will 
make any difference. Can you try the patch below ?

 From 8c1039da0d0f26cdf995156a905fc97fe7bda36c Mon Sep 17 00:00:00 2001
From: Christophe Leroy <christophe.leroy@c-s.fr>
Date: Wed, 22 May 2019 07:28:42 +0000
Subject: [PATCH] Fix Kexec

---
  arch/powerpc/include/asm/pgtable.h     | 2 ++
  arch/powerpc/kernel/machine_kexec_32.c | 4 ++++
  arch/powerpc/mm/pgtable_32.c           | 2 +-
  3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/pgtable.h 
b/arch/powerpc/include/asm/pgtable.h
index 3f53be60fb01..642eea937229 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -140,6 +140,8 @@ static inline void pte_frag_set(mm_context_t *ctx, 
void *p)
  }
  #endif

+int change_page_attr(struct page *page, int numpages, pgprot_t prot);
+
  #endif /* __ASSEMBLY__ */

  #endif /* _ASM_POWERPC_PGTABLE_H */
diff --git a/arch/powerpc/kernel/machine_kexec_32.c 
b/arch/powerpc/kernel/machine_kexec_32.c
index affe5dcce7f4..4f719501e6ae 100644
--- a/arch/powerpc/kernel/machine_kexec_32.c
+++ b/arch/powerpc/kernel/machine_kexec_32.c
@@ -54,6 +54,10 @@ void default_machine_kexec(struct kimage *image)
  	memcpy((void *)reboot_code_buffer, relocate_new_kernel,
  						relocate_new_kernel_size);

+	change_page_attr(image->control_code_page,
+			 ALIGN(KEXEC_CONTROL_PAGE_SIZE, PAGE_SIZE) >> PAGE_SHIFT,
+			 PAGE_KERNEL_TEXT);
+
  	flush_icache_range(reboot_code_buffer,
  				reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE);
  	printk(KERN_INFO "Bye!\n");
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 16ada373b32b..0e4651d803fc 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -340,7 +340,7 @@ static int __change_page_attr_noflush(struct page 
*page, pgprot_t prot)
   *
   * THIS DOES NOTHING WITH BAT MAPPINGS, DEBUG USE ONLY
   */
-static int change_page_attr(struct page *page, int numpages, pgprot_t prot)
+int change_page_attr(struct page *page, int numpages, pgprot_t prot)
  {
  	int i, err = 0;
  	unsigned long flags;
-- 
2.13.3

Re: [BISECTED] kexec regression on PowerBook G4

[Aaro Koskinen]

Hi,

On Wed, May 22, 2019 at 08:14:23AM +0200, Christophe Leroy wrote:
> Le 22/05/2019 à 00:18, Aaro Koskinen a écrit :
> >I was trying to upgrade from v5.0 -> v5.1 on PowerBook G4, but when trying
> >to kexec a kernel the system gets stuck (no errors seen on the console).
> 
> Do you mean you are trying to kexec a v5.1 kernel from a v5.0 kernel, or do
> you have a working v5.1 kernel, but kexec doesn't work with it ?

To summarize, my system's boot goes like this:

Open Firmware -> kernel A (small due to OF limit) -> (kexec) -> kernel B (big)

First both A & B were at v5.0 ==> boot works.
Then I upgraded B to v5.1 ==> boot works.
Then I upgraded A to v5.1 ==> boot fails.

So the issue must be in A. So after bisecting I got the following result:

	Kernel A with commit 93c4a162b014 ==> fails
	Kernel A with commit 93c4a162b014^1 ==> works

n >Bisected to: 93c4a162b014 ("powerpc/6xx: Store PGDIR physical address
> >in a SPRG"). This commit doesn't revert cleanly anymore but I tested
> >that the one before works OK.
> 
> Not sure that's the problem. There was a problem with that commit, but it
> was fixed by 4622a2d43101 ("powerpc/6xx: fix setup and use of
> SPRN_SPRG_PGDIR for hash32").
> You probably hit some commit between those two during bisect, that's likely
> the reason why you ended here.
> 
> Can you restart your bisect from 4622a2d43101 ?

This is not a good commit to start with, as it already gives "kernel
tried to execute exec protected page..." after the "Bye!" message.

> If you have CONFIG_SMP, maybe you should also consider taking 397d2300b08c
> ("powerpc/32s: fix flush_hash_pages() on SMP"). Stable 5.1.4 includes it.

This is UP computer and CONFIG_SMP is not set.

> >With current Linus HEAD (9c7db5004280), it gets a bit further but still
> >doesn't work: now I get an error on the console after kexec "Starting
> >new kernel! ... Bye!":
> >
> >	kernel tried to execute exec-protected page (...) - exploit attempt?
> 
> Interesting.
> 
> Do you have CONFIG_STRICT_KERNEL_RWX=y in your .config ? If so, can you
> retry without it ?

I don't set that option.

A.

Re: [BISECTED] kexec regression on PowerBook G4

[Aaro Koskinen]

Hi,

On Wed, May 22, 2019 at 07:44:56AM +0000, Christophe Leroy wrote:
> On 05/22/2019 06:14 AM, Christophe Leroy wrote:
> >Le 22/05/2019 à 00:18, Aaro Koskinen a écrit :
> >>I was trying to upgrade from v5.0 -> v5.1 on PowerBook G4, but when
> >>trying
> >>to kexec a kernel the system gets stuck (no errors seen on the console).
> >
> >Do you mean you are trying to kexec a v5.1 kernel from a v5.0 kernel, or
> >do you have a working v5.1 kernel, but kexec doesn't work with it ?
> >
> >>
> >>Bisected to: 93c4a162b014 ("powerpc/6xx: Store PGDIR physical address
> >>in a SPRG"). This commit doesn't revert cleanly anymore but I tested
> >>that the one before works OK.
> >
> >Not sure that's the problem. There was a problem with that commit, but it
> >was fixed by 4622a2d43101 ("powerpc/6xx: fix setup and use of
> >SPRN_SPRG_PGDIR for hash32").
> >You probably hit some commit between those two during bisect, that's
> >likely the reason why you ended here.
> >
> >Can you restart your bisect from 4622a2d43101 ?
> >
> >If you have CONFIG_SMP, maybe you should also consider taking 397d2300b08c
> >("powerpc/32s: fix flush_hash_pages() on SMP"). Stable 5.1.4 includes it.
> >
> >>
> >>With current Linus HEAD (9c7db5004280), it gets a bit further but still
> >>doesn't work: now I get an error on the console after kexec "Starting
> >>new kernel! ... Bye!":
> >>
> >>    kernel tried to execute exec-protected page (...) - exploit attempt?
> >
> >Interesting.
> >
> >Do you have CONFIG_STRICT_KERNEL_RWX=y in your .config ? If so, can you
> >retry without it ?
> 
> After looking at the code, I don't thing CONFIG_STRICT_KERNEL_RWX will make
> any difference. Can you try the patch below ?

Doesn't help (git refuses the patch as corrupted, so I had to do those
changes manually, but I'm pretty sure I got it right).

I still get the "kernel tried to execute exec-protected page...". What
should I try next?

A.

> From 8c1039da0d0f26cdf995156a905fc97fe7bda36c Mon Sep 17 00:00:00 2001
> From: Christophe Leroy <christophe.leroy@c-s.fr>
> Date: Wed, 22 May 2019 07:28:42 +0000
> Subject: [PATCH] Fix Kexec
> 
> ---
>  arch/powerpc/include/asm/pgtable.h     | 2 ++
>  arch/powerpc/kernel/machine_kexec_32.c | 4 ++++
>  arch/powerpc/mm/pgtable_32.c           | 2 +-
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/pgtable.h
> b/arch/powerpc/include/asm/pgtable.h
> index 3f53be60fb01..642eea937229 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -140,6 +140,8 @@ static inline void pte_frag_set(mm_context_t *ctx, void
> *p)
>  }
>  #endif
> 
> +int change_page_attr(struct page *page, int numpages, pgprot_t prot);
> +
>  #endif /* __ASSEMBLY__ */
> 
>  #endif /* _ASM_POWERPC_PGTABLE_H */
> diff --git a/arch/powerpc/kernel/machine_kexec_32.c
> b/arch/powerpc/kernel/machine_kexec_32.c
> index affe5dcce7f4..4f719501e6ae 100644
> --- a/arch/powerpc/kernel/machine_kexec_32.c
> +++ b/arch/powerpc/kernel/machine_kexec_32.c
> @@ -54,6 +54,10 @@ void default_machine_kexec(struct kimage *image)
>  	memcpy((void *)reboot_code_buffer, relocate_new_kernel,
>  						relocate_new_kernel_size);
> 
> +	change_page_attr(image->control_code_page,
> +			 ALIGN(KEXEC_CONTROL_PAGE_SIZE, PAGE_SIZE) >> PAGE_SHIFT,
> +			 PAGE_KERNEL_TEXT);
> +
>  	flush_icache_range(reboot_code_buffer,
>  				reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE);
>  	printk(KERN_INFO "Bye!\n");
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 16ada373b32b..0e4651d803fc 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -340,7 +340,7 @@ static int __change_page_attr_noflush(struct page *page,
> pgprot_t prot)
>   *
>   * THIS DOES NOTHING WITH BAT MAPPINGS, DEBUG USE ONLY
>   */
> -static int change_page_attr(struct page *page, int numpages, pgprot_t prot)
> +int change_page_attr(struct page *page, int numpages, pgprot_t prot)
>  {
>  	int i, err = 0;
>  	unsigned long flags;
> -- 
> 2.13.3

Re: [BISECTED] kexec regression on PowerBook G4

[LEROY Christophe]

Aaro Koskinen <aaro.koskinen@iki.fi> a écrit :

> Hi,
>
> On Wed, May 22, 2019 at 07:44:56AM +0000, Christophe Leroy wrote:
>> On 05/22/2019 06:14 AM, Christophe Leroy wrote:
>> >Le 22/05/2019 à 00:18, Aaro Koskinen a écrit :
>> >>I was trying to upgrade from v5.0 -> v5.1 on PowerBook G4, but when
>> >>trying
>> >>to kexec a kernel the system gets stuck (no errors seen on the console).
>> >
>> >Do you mean you are trying to kexec a v5.1 kernel from a v5.0 kernel, or
>> >do you have a working v5.1 kernel, but kexec doesn't work with it ?
>> >
>> >>
>> >>Bisected to: 93c4a162b014 ("powerpc/6xx: Store PGDIR physical address
>> >>in a SPRG"). This commit doesn't revert cleanly anymore but I tested
>> >>that the one before works OK.
>> >
>> >Not sure that's the problem. There was a problem with that commit, but it
>> >was fixed by 4622a2d43101 ("powerpc/6xx: fix setup and use of
>> >SPRN_SPRG_PGDIR for hash32").
>> >You probably hit some commit between those two during bisect, that's
>> >likely the reason why you ended here.
>> >
>> >Can you restart your bisect from 4622a2d43101 ?
>> >
>> >If you have CONFIG_SMP, maybe you should also consider taking 397d2300b08c
>> >("powerpc/32s: fix flush_hash_pages() on SMP"). Stable 5.1.4 includes it.
>> >
>> >>
>> >>With current Linus HEAD (9c7db5004280), it gets a bit further but still
>> >>doesn't work: now I get an error on the console after kexec "Starting
>> >>new kernel! ... Bye!":
>> >>
>> >>    kernel tried to execute exec-protected page (...) - exploit attempt?
>> >
>> >Interesting.
>> >
>> >Do you have CONFIG_STRICT_KERNEL_RWX=y in your .config ? If so, can you
>> >retry without it ?
>>
>> After looking at the code, I don't thing CONFIG_STRICT_KERNEL_RWX will make
>> any difference. Can you try the patch below ?
>
> Doesn't help (git refuses the patch as corrupted, so I had to do those
> changes manually, but I'm pretty sure I got it right).
>
> I still get the "kernel tried to execute exec-protected page...". What
> should I try next?

Can you provide full details of the Oops you get ? And also your System.map ?
K
Also build with CONFIG_PPC_PTDUMP and mount /sys/kernel/debug and give  
content of /sys/kernel/debug/powerpc/block_address_translation and  
.../segment_registers before the failing kexec, and also  
/sys/kernel/debug/kernel_page_tables

Thx
Christophe

>
> A.
>
>> From 8c1039da0d0f26cdf995156a905fc97fe7bda36c Mon Sep 17 00:00:00 2001
>> From: Christophe Leroy <christophe.leroy@c-s.fr>
>> Date: Wed, 22 May 2019 07:28:42 +0000
>> Subject: [PATCH] Fix Kexec
>>
>> ---
>>  arch/powerpc/include/asm/pgtable.h     | 2 ++
>>  arch/powerpc/kernel/machine_kexec_32.c | 4 ++++
>>  arch/powerpc/mm/pgtable_32.c           | 2 +-
>>  3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/pgtable.h
>> b/arch/powerpc/include/asm/pgtable.h
>> index 3f53be60fb01..642eea937229 100644
>> --- a/arch/powerpc/include/asm/pgtable.h
>> +++ b/arch/powerpc/include/asm/pgtable.h
>> @@ -140,6 +140,8 @@ static inline void pte_frag_set(mm_context_t *ctx, void
>> *p)
>>  }
>>  #endif
>>
>> +int change_page_attr(struct page *page, int numpages, pgprot_t prot);
>> +
>>  #endif /* __ASSEMBLY__ */
>>
>>  #endif /* _ASM_POWERPC_PGTABLE_H */
>> diff --git a/arch/powerpc/kernel/machine_kexec_32.c
>> b/arch/powerpc/kernel/machine_kexec_32.c
>> index affe5dcce7f4..4f719501e6ae 100644
>> --- a/arch/powerpc/kernel/machine_kexec_32.c
>> +++ b/arch/powerpc/kernel/machine_kexec_32.c
>> @@ -54,6 +54,10 @@ void default_machine_kexec(struct kimage *image)
>>  	memcpy((void *)reboot_code_buffer, relocate_new_kernel,
>>  						relocate_new_kernel_size);
>>
>> +	change_page_attr(image->control_code_page,
>> +			 ALIGN(KEXEC_CONTROL_PAGE_SIZE, PAGE_SIZE) >> PAGE_SHIFT,
>> +			 PAGE_KERNEL_TEXT);
>> +
>>  	flush_icache_range(reboot_code_buffer,
>>  				reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE);
>>  	printk(KERN_INFO "Bye!\n");
>> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
>> index 16ada373b32b..0e4651d803fc 100644
>> --- a/arch/powerpc/mm/pgtable_32.c
>> +++ b/arch/powerpc/mm/pgtable_32.c
>> @@ -340,7 +340,7 @@ static int __change_page_attr_noflush(struct page *page,
>> pgprot_t prot)
>>   *
>>   * THIS DOES NOTHING WITH BAT MAPPINGS, DEBUG USE ONLY
>>   */
>> -static int change_page_attr(struct page *page, int numpages, pgprot_t prot)
>> +int change_page_attr(struct page *page, int numpages, pgprot_t prot)
>>  {
>>  	int i, err = 0;
>>  	unsigned long flags;
>> --
>> 2.13.3

[RFC PATCH] powerpc: fix kexec failure on book3s/32

[Christophe Leroy]

Fixes: 63b2bc619565 ("powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/machine_kexec_32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/machine_kexec_32.c b/arch/powerpc/kernel/machine_kexec_32.c
index affe5dcce7f4..b6a4250b9ee0 100644
--- a/arch/powerpc/kernel/machine_kexec_32.c
+++ b/arch/powerpc/kernel/machine_kexec_32.c
@@ -54,6 +54,8 @@ void default_machine_kexec(struct kimage *image)
 	memcpy((void *)reboot_code_buffer, relocate_new_kernel,
 						relocate_new_kernel_size);
 
+	mtsrin(mfsrin(reboot_code_buffer) & ~SR_NX, reboot_code_buffer);
+
 	flush_icache_range(reboot_code_buffer,
 				reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE);
 	printk(KERN_INFO "Bye!\n");
-- 
2.13.3

Re: [BISECTED] kexec regression on PowerBook G4

[Christophe Leroy]

Hi,

Le 22/05/2019 à 23:17, Aaro Koskinen a écrit :
> Hi,
> 
> On Wed, May 22, 2019 at 10:33:35PM +0200, LEROY Christophe wrote:
>> Can you provide full details of the Oops you get ? And also your System.map ?
> 
> System.map is below. The oops log is attached as jpeg (crappy camera
> shoot, apologies, I hope it gets through) as the only way I can see it
> is the frame buffer display.
> 
>> Also build with CONFIG_PPC_PTDUMP and mount /sys/kernel/debug and give
>> content of /sys/kernel/debug/powerpc/block_address_translation and
>> .../segment_registers before the failing kexec, and also
>> /sys/kernel/debug/kernel_page_tables
> 
> The kernel that fails is essentially headless without any input access. I
> could probably do this if needed, but it's going to take a while...
> 

Ok, the Oops confirms that the error is due to executing the kexec 
control code which is located outside the kernel text area.

My yesterday's proposed change doesn't work because on book3S/32, NX 
protection is based on setting segments to NX, and using IBATs for 
kernel text.

Can you try the patch I sent out a few minutes ago ? 
(https://patchwork.ozlabs.org/patch/1103827/)

Thanks
Christophe

Re: [BISECTED] kexec regression on PowerBook G4

[Aaro Koskinen]

Hi,

On Thu, May 23, 2019 at 07:33:38AM +0200, Christophe Leroy wrote:
> Ok, the Oops confirms that the error is due to executing the kexec control
> code which is located outside the kernel text area.
> 
> My yesterday's proposed change doesn't work because on book3S/32, NX
> protection is based on setting segments to NX, and using IBATs for kernel
> text.
> 
> Can you try the patch I sent out a few minutes ago ?
> (https://patchwork.ozlabs.org/patch/1103827/)

It now crashes with "BUG: Unable to handle kernel instruction fetch"
and the faulting address is 0xef13a000.

A.

Re: [BISECTED] kexec regression on PowerBook G4

[Christophe Leroy]

Le 23/05/2019 à 19:27, Aaro Koskinen a écrit :
> Hi,
> 
> On Thu, May 23, 2019 at 07:33:38AM +0200, Christophe Leroy wrote:
>> Ok, the Oops confirms that the error is due to executing the kexec control
>> code which is located outside the kernel text area.
>>
>> My yesterday's proposed change doesn't work because on book3S/32, NX
>> protection is based on setting segments to NX, and using IBATs for kernel
>> text.
>>
>> Can you try the patch I sent out a few minutes ago ?
>> (https://patchwork.ozlabs.org/patch/1103827/)
> 
> It now crashes with "BUG: Unable to handle kernel instruction fetch"
> and the faulting address is 0xef13a000.
> 

Ok.

Can you try with both changes at the same time, ie the mtsrin(...) and 
the change_page_attr() ?

I suspect that allthough the HW is not able to check EXEC flag, the SW 
will check it before loading the hash entry.

Christophe

Re: [BISECTED] kexec regression on PowerBook G4

[Aaro Koskinen]

Hi,

On Thu, May 23, 2019 at 08:58:11PM +0200, Christophe Leroy wrote:
> Le 23/05/2019 à 19:27, Aaro Koskinen a écrit :
> >On Thu, May 23, 2019 at 07:33:38AM +0200, Christophe Leroy wrote:
> >>Ok, the Oops confirms that the error is due to executing the kexec control
> >>code which is located outside the kernel text area.
> >>
> >>My yesterday's proposed change doesn't work because on book3S/32, NX
> >>protection is based on setting segments to NX, and using IBATs for kernel
> >>text.
> >>
> >>Can you try the patch I sent out a few minutes ago ?
> >>(https://patchwork.ozlabs.org/patch/1103827/)
> >
> >It now crashes with "BUG: Unable to handle kernel instruction fetch"
> >and the faulting address is 0xef13a000.
> 
> Ok.
> 
> Can you try with both changes at the same time, ie the mtsrin(...) and the
> change_page_attr() ?
> 
> I suspect that allthough the HW is not able to check EXEC flag, the SW will
> check it before loading the hash entry.

Unfortunately still no luck... The crash is pretty much the same with both
changes.

A.

Re: [BISECTED] kexec regression on PowerBook G4

[Christophe Leroy]

Hi

Le 24/05/2019 à 00:23, Aaro Koskinen a écrit :
> Hi,
> 
> On Thu, May 23, 2019 at 08:58:11PM +0200, Christophe Leroy wrote:
>> Le 23/05/2019 à 19:27, Aaro Koskinen a écrit :
>>> On Thu, May 23, 2019 at 07:33:38AM +0200, Christophe Leroy wrote:
>>>> Ok, the Oops confirms that the error is due to executing the kexec control
>>>> code which is located outside the kernel text area.
>>>>
>>>> My yesterday's proposed change doesn't work because on book3S/32, NX
>>>> protection is based on setting segments to NX, and using IBATs for kernel
>>>> text.
>>>>
>>>> Can you try the patch I sent out a few minutes ago ?
>>>> (https://patchwork.ozlabs.org/patch/1103827/)
>>>
>>> It now crashes with "BUG: Unable to handle kernel instruction fetch"
>>> and the faulting address is 0xef13a000.
>>
>> Ok.
>>
>> Can you try with both changes at the same time, ie the mtsrin(...) and the
>> change_page_attr() ?
>>
>> I suspect that allthough the HW is not able to check EXEC flag, the SW will
>> check it before loading the hash entry.
> 
> Unfortunately still no luck... The crash is pretty much the same with both
> changes.

Right. In fact change_page_attr() does nothing because this part of RAM 
is mapped by DBATs so v_block_mapped() returns not NULL.

So, we have to set an IBAT for this area. I'll try and send you a new 
patch for that before noon (CET).

Christophe

[RFC PATCH v2] powerpc: fix kexec failure on book3s/32

[Christophe Leroy]

Fixes: 63b2bc619565 ("powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/machine_kexec_32.c | 8 ++++++++
 arch/powerpc/mm/book3s32/mmu.c         | 7 +++++--
 arch/powerpc/mm/mmu_decl.h             | 2 ++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/machine_kexec_32.c b/arch/powerpc/kernel/machine_kexec_32.c
index affe5dcce7f4..83e61a8f8468 100644
--- a/arch/powerpc/kernel/machine_kexec_32.c
+++ b/arch/powerpc/kernel/machine_kexec_32.c
@@ -15,6 +15,7 @@
 #include <asm/cacheflush.h>
 #include <asm/hw_irq.h>
 #include <asm/io.h>
+#include <mm/mmu_decl.h>
 
 typedef void (*relocate_new_kernel_t)(
 				unsigned long indirection_page,
@@ -35,6 +36,8 @@ void default_machine_kexec(struct kimage *image)
 	unsigned long page_list;
 	unsigned long reboot_code_buffer, reboot_code_buffer_phys;
 	relocate_new_kernel_t rnk;
+	unsigned long bat_size = 128 << 10;
+	unsigned long bat_mask = ~(bat_size - 1);
 
 	/* Interrupts aren't acceptable while we reboot */
 	local_irq_disable();
@@ -54,6 +57,11 @@ void default_machine_kexec(struct kimage *image)
 	memcpy((void *)reboot_code_buffer, relocate_new_kernel,
 						relocate_new_kernel_size);
 
+	printk(KERN_INFO "Reboot code buffer at %lx\n", reboot_code_buffer);
+	mtsrin(mfsrin(reboot_code_buffer) & ~SR_NX, reboot_code_buffer);
+	setibat(7, reboot_code_buffer & bat_mask, reboot_code_buffer_phys & bat_mask,
+		bat_size, PAGE_KERNEL_TEXT);
+
 	flush_icache_range(reboot_code_buffer,
 				reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE);
 	printk(KERN_INFO "Bye!\n");
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index fc073cb2c517..7124700edb0f 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -124,8 +124,8 @@ static unsigned int block_size(unsigned long base, unsigned long top)
  * of 2 between 128k and 256M.
  * Only for 603+ ...
  */
-static void setibat(int index, unsigned long virt, phys_addr_t phys,
-		    unsigned int size, pgprot_t prot)
+void setibat(int index, unsigned long virt, phys_addr_t phys,
+	     unsigned int size, pgprot_t prot)
 {
 	unsigned int bl = (size >> 17) - 1;
 	int wimgxpp;
@@ -197,6 +197,9 @@ void mmu_mark_initmem_nx(void)
 	if (cpu_has_feature(CPU_FTR_601))
 		return;
 
+	if (IS_ENABLED(CONFIG_KEXEC))
+		nb--;
+
 	for (i = 0; i < nb - 1 && base < top && top - base > (128 << 10);) {
 		size = block_size(base, top);
 		setibat(i++, PAGE_OFFSET + base, base, size, PAGE_KERNEL_TEXT);
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 7bac0aa2026a..478584d50cf2 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -103,6 +103,8 @@ void print_system_hash_info(void);
 extern void mapin_ram(void);
 extern void setbat(int index, unsigned long virt, phys_addr_t phys,
 		   unsigned int size, pgprot_t prot);
+void setibat(int index, unsigned long virt, phys_addr_t phys,
+	     unsigned int size, pgprot_t prot);
 
 extern int __map_without_bats;
 extern unsigned int rtas_data, rtas_size;
-- 
2.13.3

[RFC PATCH v2] powerpc: fix kexec failure on book3s/32

[Christophe Leroy]

Fixes: 63b2bc619565 ("powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/machine_kexec_32.c | 8 ++++++++
 arch/powerpc/mm/book3s32/mmu.c         | 7 +++++--
 arch/powerpc/mm/mmu_decl.h             | 2 ++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/machine_kexec_32.c b/arch/powerpc/kernel/machine_kexec_32.c
index affe5dcce7f4..83e61a8f8468 100644
--- a/arch/powerpc/kernel/machine_kexec_32.c
+++ b/arch/powerpc/kernel/machine_kexec_32.c
@@ -15,6 +15,7 @@
 #include <asm/cacheflush.h>
 #include <asm/hw_irq.h>
 #include <asm/io.h>
+#include <mm/mmu_decl.h>
 
 typedef void (*relocate_new_kernel_t)(
 				unsigned long indirection_page,
@@ -35,6 +36,8 @@ void default_machine_kexec(struct kimage *image)
 	unsigned long page_list;
 	unsigned long reboot_code_buffer, reboot_code_buffer_phys;
 	relocate_new_kernel_t rnk;
+	unsigned long bat_size = 128 << 10;
+	unsigned long bat_mask = ~(bat_size - 1);
 
 	/* Interrupts aren't acceptable while we reboot */
 	local_irq_disable();
@@ -54,6 +57,11 @@ void default_machine_kexec(struct kimage *image)
 	memcpy((void *)reboot_code_buffer, relocate_new_kernel,
 						relocate_new_kernel_size);
 
+	printk(KERN_INFO "Reboot code buffer at %lx\n", reboot_code_buffer);
+	mtsrin(mfsrin(reboot_code_buffer) & ~SR_NX, reboot_code_buffer);
+	setibat(7, reboot_code_buffer & bat_mask, reboot_code_buffer_phys & bat_mask,
+		bat_size, PAGE_KERNEL_TEXT);
+
 	flush_icache_range(reboot_code_buffer,
 				reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE);
 	printk(KERN_INFO "Bye!\n");
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index fc073cb2c517..7124700edb0f 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -124,8 +124,8 @@ static unsigned int block_size(unsigned long base, unsigned long top)
  * of 2 between 128k and 256M.
  * Only for 603+ ...
  */
-static void setibat(int index, unsigned long virt, phys_addr_t phys,
-		    unsigned int size, pgprot_t prot)
+void setibat(int index, unsigned long virt, phys_addr_t phys,
+	     unsigned int size, pgprot_t prot)
 {
 	unsigned int bl = (size >> 17) - 1;
 	int wimgxpp;
@@ -197,6 +197,9 @@ void mmu_mark_initmem_nx(void)
 	if (cpu_has_feature(CPU_FTR_601))
 		return;
 
+	if (IS_ENABLED(CONFIG_KEXEC))
+		nb--;
+
 	for (i = 0; i < nb - 1 && base < top && top - base > (128 << 10);) {
 		size = block_size(base, top);
 		setibat(i++, PAGE_OFFSET + base, base, size, PAGE_KERNEL_TEXT);
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 7bac0aa2026a..478584d50cf2 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -103,6 +103,8 @@ void print_system_hash_info(void);
 extern void mapin_ram(void);
 extern void setbat(int index, unsigned long virt, phys_addr_t phys,
 		   unsigned int size, pgprot_t prot);
+void setibat(int index, unsigned long virt, phys_addr_t phys,
+	     unsigned int size, pgprot_t prot);
 
 extern int __map_without_bats;
 extern unsigned int rtas_data, rtas_size;
-- 
2.13.3

Re: [BISECTED] kexec regression on PowerBook G4

[Christophe Leroy]

Le 24/05/2019 à 07:46, Christophe Leroy a écrit :
> Hi
> 
> Le 24/05/2019 à 00:23, Aaro Koskinen a écrit :
>> Hi,
>>
>> On Thu, May 23, 2019 at 08:58:11PM +0200, Christophe Leroy wrote:
>>> Le 23/05/2019 à 19:27, Aaro Koskinen a écrit :
>>>> On Thu, May 23, 2019 at 07:33:38AM +0200, Christophe Leroy wrote:
>>>>> Ok, the Oops confirms that the error is due to executing the kexec 
>>>>> control
>>>>> code which is located outside the kernel text area.
>>>>>
>>>>> My yesterday's proposed change doesn't work because on book3S/32, NX
>>>>> protection is based on setting segments to NX, and using IBATs for 
>>>>> kernel
>>>>> text.
>>>>>
>>>>> Can you try the patch I sent out a few minutes ago ?
>>>>> (https://patchwork.ozlabs.org/patch/1103827/)
>>>>
>>>> It now crashes with "BUG: Unable to handle kernel instruction fetch"
>>>> and the faulting address is 0xef13a000.
>>>
>>> Ok.
>>>
>>> Can you try with both changes at the same time, ie the mtsrin(...) 
>>> and the
>>> change_page_attr() ?
>>>
>>> I suspect that allthough the HW is not able to check EXEC flag, the 
>>> SW will
>>> check it before loading the hash entry.
>>
>> Unfortunately still no luck... The crash is pretty much the same with 
>> both
>> changes.
> 
> Right. In fact change_page_attr() does nothing because this part of RAM 
> is mapped by DBATs so v_block_mapped() returns not NULL.
> 
> So, we have to set an IBAT for this area. I'll try and send you a new 
> patch for that before noon (CET).
> 

patch sent out. In the patch I have also added a printk to print the 
buffer address, so if the problem still occurs, we'll know if the 
problem is really at the address of the buffer or if we are wrong from 
the beginning.

Christophe

Re: [BISECTED] kexec regression on PowerBook G4

[Aaro Koskinen]

Hi,

On Fri, May 24, 2019 at 08:08:36AM +0200, Christophe Leroy wrote:
> >Le 24/05/2019 à 00:23, Aaro Koskinen a écrit :
> >>Unfortunately still no luck... The crash is pretty much the same with
> >>both
> >>changes.
> >
> >Right. In fact change_page_attr() does nothing because this part of RAM is
> >mapped by DBATs so v_block_mapped() returns not NULL.
> >
> >So, we have to set an IBAT for this area. I'll try and send you a new
> >patch for that before noon (CET).
> >
> 
> patch sent out. In the patch I have also added a printk to print the buffer
> address, so if the problem still occurs, we'll know if the problem is really
> at the address of the buffer or if we are wrong from the beginning.

Reboot code buffer at ef0c3000
Bye!
BUG: Unable to handle kernel instruction fetch
Faulting instruction address: 0xef0c3000

A.

Re: [BISECTED] kexec regression on PowerBook G4

[Christophe Leroy]

Le 24/05/2019 à 09:36, Aaro Koskinen a écrit :
> Hi,
> 
> On Fri, May 24, 2019 at 08:08:36AM +0200, Christophe Leroy wrote:
>>> Le 24/05/2019 à 00:23, Aaro Koskinen a écrit :
>>>> Unfortunately still no luck... The crash is pretty much the same with
>>>> both
>>>> changes.
>>>
>>> Right. In fact change_page_attr() does nothing because this part of RAM is
>>> mapped by DBATs so v_block_mapped() returns not NULL.
>>>
>>> So, we have to set an IBAT for this area. I'll try and send you a new
>>> patch for that before noon (CET).
>>>
>>
>> patch sent out. In the patch I have also added a printk to print the buffer
>> address, so if the problem still occurs, we'll know if the problem is really
>> at the address of the buffer or if we are wrong from the beginning.
> 
> Reboot code buffer at ef0c3000
> Bye!
> BUG: Unable to handle kernel instruction fetch
> Faulting instruction address: 0xef0c3000
> 

Oops, I forgot to call update_bats() after setibat().

Can you add it and retry ?

Thanks
Christophe

Re: [BISECTED] kexec regression on PowerBook G4

[Aaro Koskinen]

Hi,

On Fri, May 24, 2019 at 09:40:30AM +0200, Christophe Leroy wrote:
> Le 24/05/2019 à 09:36, Aaro Koskinen a écrit :
> >On Fri, May 24, 2019 at 08:08:36AM +0200, Christophe Leroy wrote:
> >>>Le 24/05/2019 à 00:23, Aaro Koskinen a écrit :
> >>>>Unfortunately still no luck... The crash is pretty much the same with
> >>>>both
> >>>>changes.
> >>>
> >>>Right. In fact change_page_attr() does nothing because this part of RAM is
> >>>mapped by DBATs so v_block_mapped() returns not NULL.
> >>>
> >>>So, we have to set an IBAT for this area. I'll try and send you a new
> >>>patch for that before noon (CET).
> >>>
> >>
> >>patch sent out. In the patch I have also added a printk to print the buffer
> >>address, so if the problem still occurs, we'll know if the problem is really
> >>at the address of the buffer or if we are wrong from the beginning.
> >
> >Reboot code buffer at ef0c3000
> >Bye!
> >BUG: Unable to handle kernel instruction fetch
> >Faulting instruction address: 0xef0c3000
> >
> 
> Oops, I forgot to call update_bats() after setibat().
> 
> Can you add it and retry ?

Thanks, that was it, now it finally works!

A.

Re: [BISECTED] kexec regression on PowerBook G4

[Christophe Leroy]

Le 24/05/2019 à 15:29, Aaro Koskinen a écrit :
> Hi,
> 
> On Fri, May 24, 2019 at 09:40:30AM +0200, Christophe Leroy wrote:
>> Le 24/05/2019 à 09:36, Aaro Koskinen a écrit :
>>> On Fri, May 24, 2019 at 08:08:36AM +0200, Christophe Leroy wrote:
>>>>> Le 24/05/2019 à 00:23, Aaro Koskinen a écrit :
>>>>>> Unfortunately still no luck... The crash is pretty much the same with
>>>>>> both
>>>>>> changes.
>>>>>
>>>>> Right. In fact change_page_attr() does nothing because this part of RAM is
>>>>> mapped by DBATs so v_block_mapped() returns not NULL.
>>>>>
>>>>> So, we have to set an IBAT for this area. I'll try and send you a new
>>>>> patch for that before noon (CET).
>>>>>
>>>>
>>>> patch sent out. In the patch I have also added a printk to print the buffer
>>>> address, so if the problem still occurs, we'll know if the problem is really
>>>> at the address of the buffer or if we are wrong from the beginning.
>>>
>>> Reboot code buffer at ef0c3000
>>> Bye!
>>> BUG: Unable to handle kernel instruction fetch
>>> Faulting instruction address: 0xef0c3000
>>>
>>
>> Oops, I forgot to call update_bats() after setibat().
>>
>> Can you add it and retry ?
> 
> Thanks, that was it, now it finally works!
> 

Thanks for reporting the issue and testing.

I'll work on a clean fix patch in the begining of June.

Christophe

Re: [RFC PATCH v2] powerpc: fix kexec failure on book3s/32

[Christophe Leroy]

Le 24/05/2019 à 08:05, Christophe Leroy a écrit :
> Fixes: 63b2bc619565 ("powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>

> ---
>   arch/powerpc/kernel/machine_kexec_32.c | 8 ++++++++
>   arch/powerpc/mm/book3s32/mmu.c         | 7 +++++--
>   arch/powerpc/mm/mmu_decl.h             | 2 ++
>   3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/machine_kexec_32.c b/arch/powerpc/kernel/machine_kexec_32.c
> index affe5dcce7f4..83e61a8f8468 100644
> --- a/arch/powerpc/kernel/machine_kexec_32.c
> +++ b/arch/powerpc/kernel/machine_kexec_32.c
> @@ -15,6 +15,7 @@
>   #include <asm/cacheflush.h>
>   #include <asm/hw_irq.h>
>   #include <asm/io.h>
> +#include <mm/mmu_decl.h>
>   
>   typedef void (*relocate_new_kernel_t)(
>   				unsigned long indirection_page,
> @@ -35,6 +36,8 @@ void default_machine_kexec(struct kimage *image)
>   	unsigned long page_list;
>   	unsigned long reboot_code_buffer, reboot_code_buffer_phys;
>   	relocate_new_kernel_t rnk;
> +	unsigned long bat_size = 128 << 10;
> +	unsigned long bat_mask = ~(bat_size - 1);
>   
>   	/* Interrupts aren't acceptable while we reboot */
>   	local_irq_disable();
> @@ -54,6 +57,11 @@ void default_machine_kexec(struct kimage *image)
>   	memcpy((void *)reboot_code_buffer, relocate_new_kernel,
>   						relocate_new_kernel_size);
>   
> +	printk(KERN_INFO "Reboot code buffer at %lx\n", reboot_code_buffer);
> +	mtsrin(mfsrin(reboot_code_buffer) & ~SR_NX, reboot_code_buffer);
> +	setibat(7, reboot_code_buffer & bat_mask, reboot_code_buffer_phys & bat_mask,
> +		bat_size, PAGE_KERNEL_TEXT);

A call to update_bats() have to be added here after setibat()

Christophe

> +
>   	flush_icache_range(reboot_code_buffer,
>   				reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE);
>   	printk(KERN_INFO "Bye!\n");
> diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
> index fc073cb2c517..7124700edb0f 100644
> --- a/arch/powerpc/mm/book3s32/mmu.c
> +++ b/arch/powerpc/mm/book3s32/mmu.c
> @@ -124,8 +124,8 @@ static unsigned int block_size(unsigned long base, unsigned long top)
>    * of 2 between 128k and 256M.
>    * Only for 603+ ...
>    */
> -static void setibat(int index, unsigned long virt, phys_addr_t phys,
> -		    unsigned int size, pgprot_t prot)
> +void setibat(int index, unsigned long virt, phys_addr_t phys,
> +	     unsigned int size, pgprot_t prot)
>   {
>   	unsigned int bl = (size >> 17) - 1;
>   	int wimgxpp;
> @@ -197,6 +197,9 @@ void mmu_mark_initmem_nx(void)
>   	if (cpu_has_feature(CPU_FTR_601))
>   		return;
>   
> +	if (IS_ENABLED(CONFIG_KEXEC))
> +		nb--;
> +
>   	for (i = 0; i < nb - 1 && base < top && top - base > (128 << 10);) {
>   		size = block_size(base, top);
>   		setibat(i++, PAGE_OFFSET + base, base, size, PAGE_KERNEL_TEXT);
> diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
> index 7bac0aa2026a..478584d50cf2 100644
> --- a/arch/powerpc/mm/mmu_decl.h
> +++ b/arch/powerpc/mm/mmu_decl.h
> @@ -103,6 +103,8 @@ void print_system_hash_info(void);
>   extern void mapin_ram(void);
>   extern void setbat(int index, unsigned long virt, phys_addr_t phys,
>   		   unsigned int size, pgprot_t prot);
> +void setibat(int index, unsigned long virt, phys_addr_t phys,
> +	     unsigned int size, pgprot_t prot);
>   
>   extern int __map_without_bats;
>   extern unsigned int rtas_data, rtas_size;
>