From patchwork Thu Apr 6 15:12:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Cabaj X-Patchwork-Id: 1766193 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=f2pOJ2pP; 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 4PslP45MJcz1yZp for ; Fri, 7 Apr 2023 01:12:55 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1pkRI1-0008Ac-Fa; Thu, 06 Apr 2023 15:12:41 +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 1pkRHy-00089o-KK for kernel-team@lists.ubuntu.com; Thu, 06 Apr 2023 15:12:38 +0000 Received: from mail-yb1-f199.google.com (mail-yb1-f199.google.com [209.85.219.199]) (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 2C4743F19A for ; Thu, 6 Apr 2023 15:12:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1680793958; bh=vvmJtNN6lRCSdfTQqTHAI1IwOl4IMS3Pj+FY8EXk1OE=; h=From:To:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=f2pOJ2pPiAdAe5cR29AsA8uYRKkE3LW/gzN8xccaa6OQn8zirOMsR38V0nw4AfXl+ +zABc9Tf2u5K4Ja1+JgUqtA48TXe9ueJm2NrOW4l1APS6P4UzH7+7LVmdOaJ6vhvtW oiBdXAX8pdRGnaEZl3osaTJNyd6Dwcy4FXHS+zMfyBEx5KkmhhTKugKlE+dImhPw8i razmfYxfeZ4HX0rF9r5d19EWrqtlo5WgUffMenpWMD2RSSN072toMIrleeMIerrNzL faQO2NCoyPGRrFy5vHxVTazH7myvMuNj4flCZ9Yg8ODCNTLQ5KRs3ZuHrxKCShJVay vLeRlWfpuTAaA== Received: by mail-yb1-f199.google.com with SMTP id d83-20020a25e656000000b00b8befc985b5so3806275ybh.22 for ; Thu, 06 Apr 2023 08:12:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680793957; 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=vvmJtNN6lRCSdfTQqTHAI1IwOl4IMS3Pj+FY8EXk1OE=; b=LhNC4H43yO5qTKY5wBmZ4i1JqUnd1LyX+G1c2JhG8e6JKhpPGHbFbT09HsuNnLtAMu GDip5b+AUd3UrPcRK4Vj9yi28RDkqefPKDXnltFIksZCdz6W7TL2vBuk3V1C7KJuVqey 52Eay5gy8yzhMY84ZYMR74lSvpz9TQhtDri+yx6QcGFnQWJygj4fM5308wCBpjBX3Bj2 oci1IBEsu8LcNrnoPP3YE/HV5POk1dZORcCyNWPt7bbzXRi1CQP2F2mAd4hnRyfAClMG GDxIt3o9Y7HyKoIVVqRnV3P6hYD6R95aCndg4SX9WS5f/UGH9qadFSeVxh5obUm8rg6e C3cA== X-Gm-Message-State: AAQBX9fQXe90dp9CoIWWvefxD9RDKidraTE2rwY6jD+Ouz/yXAb048Ll D7N18pltnk5Ra5CSqoEIXRZ43Goxq6EEiOsm3RYNeP0PGxxq/N3HiR/mI/3Wtbegg2Ixnv4bMjC FIivFUvo4XwOU0X/0RFFSpAyT5yF5ydiFmSgMbPfvMHbZQ7t8Aw== X-Received: by 2002:a81:5287:0:b0:54c:995:818c with SMTP id g129-20020a815287000000b0054c0995818cmr2724102ywb.19.1680793956571; Thu, 06 Apr 2023 08:12:36 -0700 (PDT) X-Google-Smtp-Source: AKy350b1iN51Osn4yE9JzIHTHvjR3S9VSdIfcC84q0E9AjQkwVbwXPnyie+hau0lkGdLaSz6TRzLRg== X-Received: by 2002:a81:5287:0:b0:54c:995:818c with SMTP id g129-20020a815287000000b0054c0995818cmr2724044ywb.19.1680793955490; Thu, 06 Apr 2023 08:12:35 -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 80-20020a811453000000b00545a0818480sm373564ywu.16.2023.04.06.08.12.34 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Apr 2023 08:12:35 -0700 (PDT) From: John Cabaj To: kernel-team@lists.ubuntu.com Subject: [SRU][jammy][PATCH v3 1/5] kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler() Date: Thu, 6 Apr 2023 10:12:29 -0500 Message-Id: <20230406151233.405626-2-john.cabaj@canonical.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230406151233.405626-1-john.cabaj@canonical.com> References: <20230406151233.405626-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 9bcae72dda44..b01abe453744 100644 --- a/arch/arm/probes/kprobes/core.c +++ b/arch/arm/probes/kprobes/core.c @@ -392,8 +392,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 2162b6fd7251..0bc92830f1ee 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -401,8 +401,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 bd92ac376e15..63872f3df658 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 b0934a0d7aed..b33bd2498651 100644 --- a/arch/mips/kernel/kprobes.c +++ b/arch/mips/kernel/kprobes.c @@ -485,8 +485,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 7548b1d62509..e3b2bc42dbf5 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -367,7 +367,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 952d44b0610b..5fa86e54f129 100644 --- a/arch/s390/kernel/kprobes.c +++ b/arch/s390/kernel/kprobes.c @@ -343,7 +343,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 8818f3a89fef..d781b4b11ea8 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -1865,7 +1865,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; @@ -1880,7 +1879,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