[PATCH] kernel panic during kernel module load (powerpc specific part)

[roger blofeld]

--- On Wed, 6/6/12, Steffen Rumler <steffen.rumler.ext at nsn.com> wrote:

> From: Steffen Rumler <steffen.rumler.ext at nsn.com>
> Subject: [PATCH] kernel panic during kernel module load (powerpc specific part)
> To: "ext Benjamin Herrenschmidt" <benh at kernel.crashing.org>, paulus at samba.org
> Cc: "Wrobel Heinz-R39252" <r39252 at freescale.com>, "Michael Ellerman" <michael at ellerman.id.au>, "linuxppc-dev at lists.ozlabs.org" <linuxppc-dev at lists.ozlabs.org>
> Date: Wednesday, June 6, 2012, 7:37 AM
> Hi,
> 
> The patch below is intended to fix the following problem.
> 
> According to the PowerPC EABI specification, the GPR r11 is
> assigned
> the dedicated function to point to the previous stack
> frame.
> In the powerpc-specific kernel module loader, do_plt_call()
> (in arch/powerpc/kernel/module_32.c), the GPR r11 is also
> used
> to generate trampoline code.
> 
> This combination crashes the kernel, in the following case:
> 
>   + The compiler has been generated the prologue and
> epilogue,
>     which is part of the .text section.
>   + The compiler has been generated the code for the
> module init entry point,
>     part of the .init.text section (in the case it
> is marked with __init).
>   + By returning from the module init entry point, the
> epilogue is called by doing
>     a branch instruction.
>   + If the epilogue is too far away, a relative branch
> instruction cannot be applied.
>     Instead trampoline code is generated in
> do_plt_call(), in order to jump via register.
>     Unfortunately the code generated by
> do_plt_call() destroys the content of GPR r11.
>   + Because GPR r11 does not more keep the right stack
> frame pointer,
>     the kernel crashes right after the epilogue.
> 
> The fix just uses GPR r12 instead of GPR r11 for generating
> the trampoline code.
> According to the statements from Freescale, this is also
> save from EABI perspective.
> 
> I've tested the fix for kernel 2.6.33 on MPC8541.
> 
> Signed-off-by: Steffen Rumler <steffen.rumler.ext at nsn.com>
> ---
> 
> --- orig/arch/powerpc/kernel/module_32.c   
> 2012-06-06 16:04:28.956446788 +0200
> +++ new/arch/powerpc/kernel/module_32.c   
>     2012-06-06 16:04:17.746290683 +0200
> @@ -187,8 +187,8 @@
> 
>  static inline int entry_matches(struct ppc_plt_entry
> *entry, Elf32_Addr val)
>  {
> -    if (entry->jump[0] == 0x3d600000 +
> ((val + 0x8000) >> 16)
> -        &&
> entry->jump[1] == 0x396b0000 + (val & 0xffff))
> +    if (entry->jump[0] == 0x3d800000 +
> ((val + 0x8000) >> 16)
> +        &&
> entry->jump[1] == 0x398c0000 + (val & 0xffff))
>          return 1;
>      return 0;
>  }
> @@ -215,10 +215,9 @@
>          entry++;
>      }
> 
> -    /* Stolen from Paul Mackerras as well...
> */
> -    entry->jump[0] =
> 0x3d600000+((val+0x8000)>>16);    /*
> lis r11,sym at ha */
> -    entry->jump[1] = 0x396b0000 +
> (val&0xffff);    /* addi r11,r11,sym at l*/
> -    entry->jump[2] =
> 0x7d6903a6;       
>     /* mtctr r11 */
> +    entry->jump[0] =
> 0x3d800000+((val+0x8000)>>16); /* lis r12,sym at ha */
> +    entry->jump[1] = 0x398c0000 +
> (val&0xffff);     /* addi
> r12,r12,sym at l*/
> +    entry->jump[2] = 0x7d8903a6; 
>                
>   /* mtctr r12 */
>      entry->jump[3] =
> 0x4e800420;       
>     /* bctr */
> 
>      DEBUGP("Initialized plt for 0x%x at
> %p\n", val, entry);
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

Hi,

Shouldn't the corresponding ftrace code be updated to match? Perhaps like the following (untested) patch:

Signed-off-by: Roger Blofeld <blofeldus at yahoo.com>
---

--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -245,9 +245,9 @@ __ftrace_make_nop(struct module *mod,
 
 	/*
 	 * On PPC32 the trampoline looks like:
-	 *  0x3d, 0x60, 0x00, 0x00  lis r11,sym at ha
-	 *  0x39, 0x6b, 0x00, 0x00  addi r11,r11,sym at l
-	 *  0x7d, 0x69, 0x03, 0xa6  mtctr r11
+	 *  0x3d, 0x80, 0x00, 0x00  lis r12,sym at ha
+	 *  0x39, 0x8c, 0x00, 0x00  addi r12,r12,sym at l
+	 *  0x7d, 0x89, 0x03, 0xa6  mtctr r12
 	 *  0x4e, 0x80, 0x04, 0x20  bctr
 	 */
 
@@ -262,9 +262,9 @@ __ftrace_make_nop(struct module *mod,
 	pr_devel(" %08x %08x ", jmp[0], jmp[1]);
 
 	/* verify that this is what we expect it to be */
-	if (((jmp[0] & 0xffff0000) != 0x3d600000) ||
-	    ((jmp[1] & 0xffff0000) != 0x396b0000) ||
-	    (jmp[2] != 0x7d6903a6) ||
+	if (((jmp[0] & 0xffff0000) != 0x3d800000) ||
+	    ((jmp[1] & 0xffff0000) != 0x398c0000) ||
+	    (jmp[2] != 0x7d8903a6) ||
 	    (jmp[3] != 0x4e800420)) {
 		printk(KERN_ERR "Not a trampoline\n");
 		return -EINVAL;