powerpc: ftrace, fix cast aliasing and add code verification

Impact: clean up and robustness addition

This patch addresses the comments made by Paul Mackerras.
It removes the type casting between unsigned int and unsigned char
pointers, and replaces them with a use of all unsigned int.

Verification that the jump is indeed made to a trampoline has also
been added.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 3271cd698e4c5e3c10feb264388f7e63a9594e6b..ea454a004406311abf0cb6de99e27133ac5b4207 100644 (file)
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -162,26 +162,25 @@ static int
 __ftrace_make_nop(struct module *mod,
                  struct dyn_ftrace *rec, unsigned long addr)
 {
-       unsigned char replaced[MCOUNT_INSN_SIZE * 2];
-       unsigned int *op = (unsigned *)&replaced;
-       unsigned char jmp[8];
-       unsigned long *ptr = (unsigned long *)&jmp;
+       unsigned int op;
+       unsigned int jmp[5];
+       unsigned long ptr;
        unsigned long ip = rec->ip;
        unsigned long tramp;
        int offset;
 
        /* read where this goes */
-       if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE))
+       if (probe_kernel_read(&op, (void *)ip, sizeof(int)))
                return -EFAULT;
 
        /* Make sure that that this is still a 24bit jump */
-       if (!is_bl_op(*op)) {
-               printk(KERN_ERR "Not expected bl: opcode is %x\n", *op);
+       if (!is_bl_op(op)) {
+               printk(KERN_ERR "Not expected bl: opcode is %x\n", op);
                return -EINVAL;
        }
 
        /* lets find where the pointer goes */
-       tramp = find_bl_target(ip, *op);
+       tramp = find_bl_target(ip, op);
 
        /*
         * On PPC64 the trampoline looks like:
@@ -200,19 +199,25 @@ __ftrace_make_nop(struct module *mod,
        DEBUGP("ip:%lx jumps to %lx r2: %lx", ip, tramp, mod->arch.toc);
 
        /* Find where the trampoline jumps to */
-       if (probe_kernel_read(jmp, (void *)tramp, 8)) {
+       if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
                printk(KERN_ERR "Failed to read %lx\n", tramp);
                return -EFAULT;
        }
 
-       DEBUGP(" %08x %08x",
-              (unsigned)(*ptr >> 32),
-              (unsigned)*ptr);
+       DEBUGP(" %08x %08x", jmp[0], jmp[1]);
+
+       /* verify that this is what we expect it to be */
+       if (((jmp[0] & 0xffff0000) != 0x3d820000) ||
+           ((jmp[1] & 0xffff0000) != 0x398c0000) ||
+           (jmp[2] != 0xf8410028) ||
+           (jmp[3] != 0xe96c0020) ||
+           (jmp[4] != 0xe84c0028)) {
+               printk(KERN_ERR "Not a trampoline\n");
+               return -EINVAL;
+       }
 
-       offset = (unsigned)jmp[2] << 24 |
-               (unsigned)jmp[3] << 16 |
-               (unsigned)jmp[6] << 8 |
-               (unsigned)jmp[7];
+       offset = (unsigned)((unsigned short)jmp[0]) << 16 |
+               (unsigned)((unsigned short)jmp[1]);
 
        DEBUGP(" %x ", offset);
 
@@ -225,13 +230,13 @@ __ftrace_make_nop(struct module *mod,
                return -EFAULT;
        }
 
-       DEBUGP(" %08x %08x\n",
-              (unsigned)(*ptr >> 32),
-              (unsigned)*ptr);
+       DEBUGP(" %08x %08x\n", jmp[0], jmp[1]);
+
+       ptr = ((unsigned long)jmp[0] << 32) + jmp[1];
 
        /* This should match what was called */
-       if (*ptr != GET_ADDR(addr)) {
-               printk(KERN_ERR "addr does not match %lx\n", *ptr);
+       if (ptr != GET_ADDR(addr)) {
+               printk(KERN_ERR "addr does not match %lx\n", ptr);
                return -EINVAL;
        }
 
@@ -240,11 +245,11 @@ __ftrace_make_nop(struct module *mod,
         *  0xe8, 0x41, 0x00, 0x28   ld r2,40(r1)
         * This needs to be turned to a nop too.
         */
-       if (probe_kernel_read(replaced, (void *)(ip+4), MCOUNT_INSN_SIZE))
+       if (probe_kernel_read(&op, (void *)(ip+4), MCOUNT_INSN_SIZE))
                return -EFAULT;
 
-       if (*op != 0xe8410028) {
-               printk(KERN_ERR "Next line is not ld! (%08x)\n", *op);
+       if (op != 0xe8410028) {
+               printk(KERN_ERR "Next line is not ld! (%08x)\n", op);
                return -EINVAL;
        }
 
@@ -261,9 +266,9 @@ __ftrace_make_nop(struct module *mod,
         *   ld r2,40(r1)
         *  1:
         */
-       op[0] = 0x48000008;     /* b +8 */
+       op = 0x48000008;        /* b +8 */
 
-       if (probe_kernel_write((void *)ip, replaced, MCOUNT_INSN_SIZE))
+       if (probe_kernel_write((void *)ip, &op, MCOUNT_INSN_SIZE))
                return -EPERM;
 
        return 0;
@@ -274,46 +279,52 @@ static int
 __ftrace_make_nop(struct module *mod,
                  struct dyn_ftrace *rec, unsigned long addr)
 {
-       unsigned char replaced[MCOUNT_INSN_SIZE];
-       unsigned int *op = (unsigned *)&replaced;
-       unsigned char jmp[8];
-       unsigned int *ptr = (unsigned int *)&jmp;
+       unsigned int op;
+       unsigned int jmp[4];
        unsigned long ip = rec->ip;
        unsigned long tramp;
-       int offset;
 
-       if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE))
+       if (probe_kernel_read(&op, (void *)ip, MCOUNT_INSN_SIZE))
                return -EFAULT;
 
        /* Make sure that that this is still a 24bit jump */
-       if (!is_bl_op(*op)) {
-               printk(KERN_ERR "Not expected bl: opcode is %x\n", *op);
+       if (!is_bl_op(op)) {
+               printk(KERN_ERR "Not expected bl: opcode is %x\n", op);
                return -EINVAL;
        }
 
        /* lets find where the pointer goes */
-       tramp = find_bl_target(ip, *op);
+       tramp = find_bl_target(ip, op);
 
        /*
         * On PPC32 the trampoline looks like:
-        * lis r11,sym@ha
-        * addi r11,r11,sym@l
-        * mtctr r11
-        * bctr
+        *  0x3d, 0x60, 0x00, 0x00  lis r11,sym@ha
+        *  0x39, 0x6b, 0x00, 0x00  addi r11,r11,sym@l
+        *  0x7d, 0x69, 0x03, 0xa6  mtctr r11
+        *  0x4e, 0x80, 0x04, 0x20  bctr
         */
 
        DEBUGP("ip:%lx jumps to %lx", ip, tramp);
 
        /* Find where the trampoline jumps to */
-       if (probe_kernel_read(jmp, (void *)tramp, 8)) {
+       if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
                printk(KERN_ERR "Failed to read %lx\n", tramp);
                return -EFAULT;
        }
 
-       DEBUGP(" %08x %08x ", ptr[0], ptr[1]);
+       DEBUGP(" %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) ||
+           (jmp[3] != 0x4e800420)) {
+               printk(KERN_ERR "Not a trampoline\n");
+               return -EINVAL;
+       }
 
-       tramp = (ptr[1] & 0xffff) |
-               ((ptr[0] & 0xffff) << 16);
+       tramp = (jmp[1] & 0xffff) |
+               ((jmp[0] & 0xffff) << 16);
        if (tramp & 0x8000)
                tramp -= 0x10000;
 
@@ -326,9 +337,9 @@ __ftrace_make_nop(struct module *mod,
                return -EINVAL;
        }
 
-       op[0] = PPC_NOP_INSTR;
+       op = PPC_NOP_INSTR;
 
-       if (probe_kernel_write((void *)ip, replaced, MCOUNT_INSN_SIZE))
+       if (probe_kernel_write((void *)ip, &op, MCOUNT_INSN_SIZE))
                return -EPERM;
 
        return 0;
@@ -384,13 +395,12 @@ int ftrace_make_nop(struct module *mod,
 static int
 __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
-       unsigned char replaced[MCOUNT_INSN_SIZE * 2];
-       unsigned int *op = (unsigned *)&replaced;
+       unsigned int op[2];
        unsigned long ip = rec->ip;
        unsigned long offset;
 
        /* read where this goes */
-       if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE * 2))
+       if (probe_kernel_read(op, (void *)ip, MCOUNT_INSN_SIZE * 2))
                return -EFAULT;
 
        /*
@@ -425,7 +435,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 
        DEBUGP("write to %lx\n", rec->ip);
 
-       if (probe_kernel_write((void *)ip, replaced, MCOUNT_INSN_SIZE * 2))
+       if (probe_kernel_write((void *)ip, op, MCOUNT_INSN_SIZE * 2))
                return -EPERM;
 
        return 0;
@@ -434,18 +444,17 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 static int
 __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
-       unsigned char replaced[MCOUNT_INSN_SIZE];
-       unsigned int *op = (unsigned *)&replaced;
+       unsigned int op;
        unsigned long ip = rec->ip;
        unsigned long offset;
 
        /* read where this goes */
-       if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE))
+       if (probe_kernel_read(&op, (void *)ip, MCOUNT_INSN_SIZE))
                return -EFAULT;
 
        /* It should be pointing to a nop */
-       if (op[0] != PPC_NOP_INSTR) {
-               printk(KERN_ERR "Expected NOP but have %x\n", op[0]);
+       if (op != PPC_NOP_INSTR) {
+               printk(KERN_ERR "Expected NOP but have %x\n", op);
                return -EINVAL;
        }
 
@@ -465,11 +474,11 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
        }
 
        /* Set to "bl addr" */
-       op[0] = branch_offset(offset);
+       op = branch_offset(offset);
 
        DEBUGP("write to %lx\n", rec->ip);
 
-       if (probe_kernel_write((void *)ip, replaced, MCOUNT_INSN_SIZE))
+       if (probe_kernel_write((void *)ip, &op, MCOUNT_INSN_SIZE))
                return -EPERM;
 
        return 0;