x86/kprobes: Verify stack frame on kretprobe [Linux 4.4.179]

x86/kprobes: Verify stack frame on kretprobe [Linux 4.4.179]

This Linux kernel change "x86/kprobes: Verify stack frame on kretprobe" is included in the Linux 4.4.179 release. This change is authored by Masami Hiramatsu <mhiramat [at] kernel.org> on Sun Feb 24 01:49:52 2019 +0900. The commit for this change in Linux stable tree is 3dda8d2 (patch) which is from upstream commit 3ff9c07. The same Linux upstream change may have been applied to various maintained Linux releases and you can find all Linux releases containing changes from upstream 3ff9c07.

x86/kprobes: Verify stack frame on kretprobe

commit 3ff9c075cc767b3060bdac12da72fc94dd7da1b8 upstream.

Verify the stack frame pointer on kretprobe trampoline handler,
If the stack frame pointer does not match, it skips the wrong
entry and tries to find correct one.

This can happen if user puts the kretprobe on the function
which can be used in the path of ftrace user-function call.
Such functions should not be probed, so this adds a warning
message that reports which function should be blacklisted.

Tested-by: Andrea Righi <righi.andrea@gmail.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/155094059185.6137.15527904013362842072.stgit@devbox
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

There are 27 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 arch/x86/kernel/kprobes/core.c | 26 ++++++++++++++++++++++++++
 include/linux/kprobes.h        |  1 +
 2 files changed, 27 insertions(+)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index c6f466d..a9fc229 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -541,6 +541,7 @@ void arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs)
    unsigned long *sara = stack_addr(regs);

    ri->ret_addr = (kprobe_opcode_t *) *sara;
+   ri->fp = sara;

    /* Replace the return addr with trampoline addr */
    *sara = (unsigned long) &kretprobe_trampoline;
@@ -742,15 +743,21 @@ __visible __used void *trampoline_handler(struct pt_regs *regs)
    unsigned long flags, orig_ret_address = 0;
    unsigned long trampoline_address = (unsigned long)&kretprobe_trampoline;
    kprobe_opcode_t *correct_ret_addr = NULL;
+   void *frame_pointer;
+   bool skipped = false;

    INIT_HLIST_HEAD(&empty_rp);
    kretprobe_hash_lock(current, &head, &flags);
    /* fixup registers */
 #ifdef CONFIG_X86_64
    regs->cs = __KERNEL_CS;
+   /* On x86-64, we use pt_regs->sp for return address holder. */
+   frame_pointer = &regs->sp;
 #else
    regs->cs = __KERNEL_CS | get_kernel_rpl();
    regs->gs = 0;
+   /* On x86-32, we use pt_regs->flags for return address holder. */
+   frame_pointer = &regs->flags;
 #endif
    regs->ip = trampoline_address;
    regs->orig_ax = ~0UL;
@@ -772,8 +779,25 @@ __visible __used void *trampoline_handler(struct pt_regs *regs)
        if (ri->task != current)
            /* another task is sharing our hash bucket */
            continue;
+       /*
+        * Return probes must be pushed on this hash list correct
+        * order (same as return order) so that it can be poped
+        * correctly. However, if we find it is pushed it incorrect
+        * order, this means we find a function which should not be
+        * probed, because the wrong order entry is pushed on the
+        * path of processing other kretprobe itself.
+        */
+       if (ri->fp != frame_pointer) {
+           if (!skipped)
+               pr_warn("kretprobe is stacked incorrectly. Trying to fixup.\n");
+           skipped = true;
+           continue;
+       }

        orig_ret_address = (unsigned long)ri->ret_addr;
+       if (skipped)
+           pr_warn("%ps must be blacklisted because of incorrect kretprobe order\n",
+               ri->rp->kp.addr);

        if (orig_ret_address != trampoline_address)
            /*
@@ -791,6 +815,8 @@ __visible __used void *trampoline_handler(struct pt_regs *regs)
        if (ri->task != current)
            /* another task is sharing our hash bucket */
            continue;
+       if (ri->fp != frame_pointer)
+           continue;

        orig_ret_address = (unsigned long)ri->ret_addr;
        if (ri->rp && ri->rp->handler) {
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index e233925..cb527c7 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -197,6 +197,7 @@ struct kretprobe_instance {
    struct kretprobe *rp;
    kprobe_opcode_t *ret_addr;
    struct task_struct *task;
+   void *fp;
    char data[0];
 };

Leave a Reply

Your email address will not be published. Required fields are marked *