From patchwork Fri Mar 31 14:00:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Cabaj X-Patchwork-Id: 1763789 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=wAhcOYKC; dkim-atps=neutral Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Pp24n05nzz1yZ0 for ; Sat, 1 Apr 2023 01:00:57 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1piFJD-0002Xl-JH; Fri, 31 Mar 2023 14:00:51 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1piFJ8-0002Vn-HW for kernel-team@lists.ubuntu.com; Fri, 31 Mar 2023 14:00:46 +0000 Received: from mail-yw1-f200.google.com (mail-yw1-f200.google.com [209.85.128.200]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 726B13F202 for ; Fri, 31 Mar 2023 14:00:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1680271245; bh=yt684Yf8ZAsYNquudmDJIRWpphnnxPiH4Ca/1WHJXzE=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=wAhcOYKCP+mSWNziwiPzKKQacKZSi6AnvjhUgfhq/PgS6HQoEdti3xA2VHrv/QwRU xCQqGy1jlfihKe/r/DxbI4MtzMsns2ufYg/jYL94EIGygICGuXEjmKeCxumSBSkTLj GfikENC80qtwpU4lcgRhmkTz2nG/l7VeMJ0HBKeLFGcgaGf8IOjd9K3btTlPxJTTu/ tLKT1FCsyEc6vDNDZxQLV4bYTe/kwEVV5qdIrQX/7LgkBsYxfoeAtzxnuLlTasek4k 8WTB27gvOwj/1Udm3hghNIhCliN3KNmg4bqVxBuphcs+fHAiXtaZv1XHMYYEXam+v9 wGsABS3U3LuYw== Received: by mail-yw1-f200.google.com with SMTP id 00721157ae682-536a4eba107so222007307b3.19 for ; Fri, 31 Mar 2023 07:00:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680271244; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yt684Yf8ZAsYNquudmDJIRWpphnnxPiH4Ca/1WHJXzE=; b=UbZ7S/+cdUZ/P4QkShPoppTDZJWmtbv4m4hhyTEOmc+UGN3nVtVSSOvcYOZ9v6InpP sH+OVU8G2HII4jDBLUUjM0pfRZ+5hnn4Yv5Y4w0gOKr5yypOy19P+UdTmjID9+VdDU88 gZjZ8lqusfEoYnolbiQTUa+VNk6f3JPZfi2Ybx+fXDSHjmTYlcNCTEEAlpFM4pN1T/de zt3nycptxJmBoryU9SbmQsPCdtmdG/nBza+9thVSq7z/tqj+BAvTLk3kXjvN6LzaEZT9 QmVCMAWSUA0lRxFvEoERubOndySihfVFSWa7Dhg0gIkH/cldziI/vjlw0rz7fMxTED8i p1dQ== X-Gm-Message-State: AAQBX9cO+HhnZKKOTE/xZzjHvhQ/zyS9g1AIqCj28UexkxhUUZQswYv0 1tQLTFzSUZf39D58ii0RoisMf9DlqfIP6eTn55AOpybVZNPodnf2NPnclVNL7EpdYCB10mQSVa2 FM/Vly1hAvZ2cDy5YodLsOd+22kkU9CkXSpjjyzuNcxXun1OAFQ== X-Received: by 2002:a25:2654:0:b0:a48:7b0c:2f58 with SMTP id m81-20020a252654000000b00a487b0c2f58mr25387229ybm.47.1680271244036; Fri, 31 Mar 2023 07:00:44 -0700 (PDT) X-Google-Smtp-Source: AKy350b9oZJYcWY2Y3aamxu5zBVN2OEkFX8nb4jczUbwezUD0ImSpkCheoYPiCbXwMZen00uKleuGA== X-Received: by 2002:a25:2654:0:b0:a48:7b0c:2f58 with SMTP id m81-20020a252654000000b00a487b0c2f58mr25387178ybm.47.1680271243513; Fri, 31 Mar 2023 07:00:43 -0700 (PDT) Received: from smtp.gmail.com (h69-130-246-116.mdtnwi.broadband.dynamic.tds.net. [69.130.246.116]) by smtp.gmail.com with ESMTPSA id a5-20020a256605000000b00b7e0d092f91sm584336ybc.18.2023.03.31.07.00.43 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 31 Mar 2023 07:00:43 -0700 (PDT) From: John Cabaj To: kernel-team@lists.ubuntu.com Subject: [SRU][jammy][PATCH v2 1/5] kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler() Date: Fri, 31 Mar 2023 09:00:37 -0500 Message-Id: <20230331140041.2112510-2-john.cabaj@canonical.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230331140041.2112510-1-john.cabaj@canonical.com> References: <20230331140041.2112510-1-john.cabaj@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Masami Hiramatsu BugLink: https://bugs.launchpad.net/bugs/2013603 (Kernel livepatch ftrace graph fix) The __kretprobe_trampoline_handler() callback, called from low level arch kprobes methods, has the 'trampoline_address' parameter, which is entirely superfluous as it basically just replicates: dereference_kernel_function_descriptor(kretprobe_trampoline) In fact we had bugs in arch code where it wasn't replicated correctly. So remove this superfluous parameter and use kretprobe_trampoline_addr() instead. Link: https://lkml.kernel.org/r/163163044546.489837.13505751885476015002.stgit@devnote2 Signed-off-by: Masami Hiramatsu Tested-by: Andrii Nakryiko Signed-off-by: Steven Rostedt (VMware) (cherry picked from commit 96fed8ac2bb64ab45497fdd8e3d390165b7a9be8) Signed-off-by: John Cabaj --- arch/arc/kernel/kprobes.c | 2 +- arch/arm/probes/kprobes/core.c | 3 +-- arch/arm64/kernel/probes/kprobes.c | 3 +-- arch/csky/kernel/probes/kprobes.c | 2 +- arch/ia64/kernel/kprobes.c | 5 ++--- arch/mips/kernel/kprobes.c | 3 +-- arch/parisc/kernel/kprobes.c | 4 ++-- arch/powerpc/kernel/kprobes.c | 2 +- arch/riscv/kernel/probes/kprobes.c | 2 +- arch/s390/kernel/kprobes.c | 2 +- arch/sh/kernel/kprobes.c | 2 +- arch/sparc/kernel/kprobes.c | 2 +- arch/x86/include/asm/kprobes.h | 1 - arch/x86/kernel/kprobes/core.c | 2 +- include/linux/kprobes.h | 18 +++++++++++++----- kernel/kprobes.c | 3 +-- 16 files changed, 29 insertions(+), 27 deletions(-) diff --git a/arch/arc/kernel/kprobes.c b/arch/arc/kernel/kprobes.c index 5f0415fc7328..3cee75c87f97 100644 --- a/arch/arc/kernel/kprobes.c +++ b/arch/arc/kernel/kprobes.c @@ -381,7 +381,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, static int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) { - regs->ret = __kretprobe_trampoline_handler(regs, &kretprobe_trampoline, NULL); + regs->ret = __kretprobe_trampoline_handler(regs, NULL); /* By returning a non zero value, we are telling the kprobe handler * that we don't want the post_handler to run diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c index 9d8634e2f12f..93d3c3e9575a 100644 --- a/arch/arm/probes/kprobes/core.c +++ b/arch/arm/probes/kprobes/core.c @@ -390,8 +390,7 @@ void __naked __kprobes kretprobe_trampoline(void) /* Called from kretprobe_trampoline */ static __used __kprobes void *trampoline_handler(struct pt_regs *regs) { - return (void *)kretprobe_trampoline_handler(regs, &kretprobe_trampoline, - (void *)regs->ARM_fp); + return (void *)kretprobe_trampoline_handler(regs, (void *)regs->ARM_fp); } void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index b7404dba0d62..ade4975eeb8c 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -398,8 +398,7 @@ int __init arch_populate_kprobe_blacklist(void) void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs) { - return (void *)kretprobe_trampoline_handler(regs, &kretprobe_trampoline, - (void *)kernel_stack_pointer(regs)); + return (void *)kretprobe_trampoline_handler(regs, (void *)kernel_stack_pointer(regs)); } void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, diff --git a/arch/csky/kernel/probes/kprobes.c b/arch/csky/kernel/probes/kprobes.c index 584ed9f36290..495eb28d03ff 100644 --- a/arch/csky/kernel/probes/kprobes.c +++ b/arch/csky/kernel/probes/kprobes.c @@ -390,7 +390,7 @@ int __init arch_populate_kprobe_blacklist(void) void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs) { - return (void *)kretprobe_trampoline_handler(regs, &kretprobe_trampoline, NULL); + return (void *)kretprobe_trampoline_handler(regs, NULL); } void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, diff --git a/arch/ia64/kernel/kprobes.c b/arch/ia64/kernel/kprobes.c index d4048518a1d7..7802c3e1a4cb 100644 --- a/arch/ia64/kernel/kprobes.c +++ b/arch/ia64/kernel/kprobes.c @@ -392,14 +392,13 @@ static void __kprobes set_current_kprobe(struct kprobe *p, __this_cpu_write(current_kprobe, p); } -static void kretprobe_trampoline(void) +void kretprobe_trampoline(void) { } int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) { - regs->cr_iip = __kretprobe_trampoline_handler(regs, - dereference_function_descriptor(kretprobe_trampoline), NULL); + regs->cr_iip = __kretprobe_trampoline_handler(regs, NULL); /* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler diff --git a/arch/mips/kernel/kprobes.c b/arch/mips/kernel/kprobes.c index 75bff0f77319..21a4fda1e2cb 100644 --- a/arch/mips/kernel/kprobes.c +++ b/arch/mips/kernel/kprobes.c @@ -486,8 +486,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, static int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) { - instruction_pointer(regs) = __kretprobe_trampoline_handler(regs, - kretprobe_trampoline, NULL); + instruction_pointer(regs) = __kretprobe_trampoline_handler(regs, NULL); /* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler diff --git a/arch/parisc/kernel/kprobes.c b/arch/parisc/kernel/kprobes.c index 6d21a515eea5..4a35ac6e2ca2 100644 --- a/arch/parisc/kernel/kprobes.c +++ b/arch/parisc/kernel/kprobes.c @@ -175,7 +175,7 @@ int __kprobes parisc_kprobe_ss_handler(struct pt_regs *regs) return 1; } -static inline void kretprobe_trampoline(void) +void kretprobe_trampoline(void) { asm volatile("nop"); asm volatile("nop"); @@ -193,7 +193,7 @@ static int __kprobes trampoline_probe_handler(struct kprobe *p, { unsigned long orig_ret_address; - orig_ret_address = __kretprobe_trampoline_handler(regs, trampoline_p.addr, NULL); + orig_ret_address = __kretprobe_trampoline_handler(regs, NULL); instruction_pointer_set(regs, orig_ret_address); return 1; diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c index 61552f57db0b..ae4a464265ac 100644 --- a/arch/powerpc/kernel/kprobes.c +++ b/arch/powerpc/kernel/kprobes.c @@ -423,7 +423,7 @@ static int trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) { unsigned long orig_ret_address; - orig_ret_address = __kretprobe_trampoline_handler(regs, &kretprobe_trampoline, NULL); + orig_ret_address = __kretprobe_trampoline_handler(regs, NULL); /* * We get here through one of two paths: * 1. by taking a trap -> kprobe_handler() -> here diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c index 00088dc6da4b..e76c77bdce4d 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -348,7 +348,7 @@ int __init arch_populate_kprobe_blacklist(void) void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs) { - return (void *)kretprobe_trampoline_handler(regs, &kretprobe_trampoline, NULL); + return (void *)kretprobe_trampoline_handler(regs, NULL); } void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c index 52d056a5f89f..5643eda96060 100644 --- a/arch/s390/kernel/kprobes.c +++ b/arch/s390/kernel/kprobes.c @@ -341,7 +341,7 @@ static void __used kretprobe_trampoline_holder(void) */ static int trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) { - regs->psw.addr = __kretprobe_trampoline_handler(regs, &kretprobe_trampoline, NULL); + regs->psw.addr = __kretprobe_trampoline_handler(regs, NULL); /* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler diff --git a/arch/sh/kernel/kprobes.c b/arch/sh/kernel/kprobes.c index 1c7f358ef0be..8e76a35e6e33 100644 --- a/arch/sh/kernel/kprobes.c +++ b/arch/sh/kernel/kprobes.c @@ -303,7 +303,7 @@ static void __used kretprobe_trampoline_holder(void) */ int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) { - regs->pc = __kretprobe_trampoline_handler(regs, &kretprobe_trampoline, NULL); + regs->pc = __kretprobe_trampoline_handler(regs, NULL); return 1; } diff --git a/arch/sparc/kernel/kprobes.c b/arch/sparc/kernel/kprobes.c index 4c05a4ee6a0e..401534236c2e 100644 --- a/arch/sparc/kernel/kprobes.c +++ b/arch/sparc/kernel/kprobes.c @@ -451,7 +451,7 @@ static int __kprobes trampoline_probe_handler(struct kprobe *p, { unsigned long orig_ret_address = 0; - orig_ret_address = __kretprobe_trampoline_handler(regs, &kretprobe_trampoline, NULL); + orig_ret_address = __kretprobe_trampoline_handler(regs, NULL); regs->tpc = orig_ret_address; regs->tnpc = orig_ret_address + 4; diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h index bd7f5886a789..71ea2eab43d5 100644 --- a/arch/x86/include/asm/kprobes.h +++ b/arch/x86/include/asm/kprobes.h @@ -49,7 +49,6 @@ extern __visible kprobe_opcode_t optprobe_template_end[]; extern const int kretprobe_blacklist_size; void arch_remove_kprobe(struct kprobe *p); -asmlinkage void kretprobe_trampoline(void); extern void arch_kprobe_override_function(struct pt_regs *regs); diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index c4b618d0b16a..a309b5e40f10 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -1072,7 +1072,7 @@ __used __visible void *trampoline_handler(struct pt_regs *regs) regs->ip = (unsigned long)&kretprobe_trampoline; regs->orig_ax = ~0UL; - return (void *)kretprobe_trampoline_handler(regs, &kretprobe_trampoline, ®s->sp); + return (void *)kretprobe_trampoline_handler(regs, ®s->sp); } NOKPROBE_SYMBOL(trampoline_handler); diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index ef8c7accbc68..3d764f3055f6 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -199,15 +199,23 @@ extern void arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs); extern int arch_trampoline_kprobe(struct kprobe *p); +void kretprobe_trampoline(void); +/* + * Since some architecture uses structured function pointer, + * use dereference_function_descriptor() to get real function address. + */ +static nokprobe_inline void *kretprobe_trampoline_addr(void) +{ + return dereference_kernel_function_descriptor(kretprobe_trampoline); +} + /* If the trampoline handler called from a kprobe, use this version */ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, - void *trampoline_address, - void *frame_pointer); + void *frame_pointer); static nokprobe_inline unsigned long kretprobe_trampoline_handler(struct pt_regs *regs, - void *trampoline_address, - void *frame_pointer) + void *frame_pointer) { unsigned long ret; /* @@ -216,7 +224,7 @@ unsigned long kretprobe_trampoline_handler(struct pt_regs *regs, * be running at this point. */ kprobe_busy_begin(); - ret = __kretprobe_trampoline_handler(regs, trampoline_address, frame_pointer); + ret = __kretprobe_trampoline_handler(regs, frame_pointer); kprobe_busy_end(); return ret; diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 23af2f8e8563..7da4a5048eb7 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -1867,7 +1867,6 @@ unsigned long __weak arch_deref_entry_point(void *entry) #ifdef CONFIG_KRETPROBES unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, - void *trampoline_address, void *frame_pointer) { kprobe_opcode_t *correct_ret_addr = NULL; @@ -1882,7 +1881,7 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, BUG_ON(ri->fp != frame_pointer); - if (ri->ret_addr != trampoline_address) { + if (ri->ret_addr != kretprobe_trampoline_addr()) { correct_ret_addr = ri->ret_addr; /* * This is the real return address. Any other