Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 2 Mar 2022 21:51:36 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        alexei.starovoitov@gmail.com, alyssa.milburn@intel.com,
        andrew.cooper3@citrix.com, hjl.tools@gmail.com,
        joao@overdrivepizza.com, jpoimboe@redhat.com,
        keescook@chromium.org, linux-kernel@vger.kernel.org,
        mark.rutland@arm.com, mbenes@suse.cz, ndesaulniers@google.com,
        samitolvanen@google.com, x86@kernel.org
Subject: Re: [PATCH v2 12/39] x86/ibt,ftrace: Search for __fentry__ location
Message-ID: <Yh/Y2FHw90m00owK@hirez.programming.kicks-ass.net>
References: <20220224105847.5c899324@gandalf.local.home>
 <20220225103449.6084e5314273556f35107cd9@kernel.org>
 <20220224211919.0612a3f6@rorschach.local.home>
 <20220225192008.d7a4f2b5558fe43c95909777@kernel.org>
 <20220225083647.12ceb54b@gandalf.local.home>
 <1646159447.ngbqgzj71t.naveen@linux.ibm.com>
 <20220301142016.22e787fb@gandalf.local.home>
 <Yh9vF8REB1JlhQCJ@hirez.programming.kicks-ass.net>
 <20220302110138.6d2abcec@gandalf.local.home>
 <20220302144716.1772020c@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220302144716.1772020c@gandalf.local.home>
Precedence: bulk

On Wed, Mar 02, 2022 at 02:47:16PM -0500, Steven Rostedt wrote:
> Note, I just pulled this patch, and I hit this warning:
> 
> WARNING: CPU: 0 PID: 6965 at arch/x86/kernel/kprobes/core.c:205 recover_probed_instruction+0x8f/0xa0
> 
> static unsigned long
> __recover_probed_insn(kprobe_opcode_t *buf, unsigned long addr)
> {
>         struct kprobe *kp;
>         unsigned long faddr;
> 
>         kp = get_kprobe((void *)addr);
>         faddr = ftrace_location(addr);
>         /*
>          * Addresses inside the ftrace location are refused by
>          * arch_check_ftrace_location(). Something went terribly wrong
>          * if such an address is checked here.
>          */
>         if (WARN_ON(faddr && faddr != addr))  <<---- HERE
>                 return 0UL;

Ha! so a bunch of IRC later I figured out how it is possible you hit
this with just the patch on and how I legitimately hit this and what to
do about it.

Your problem seems to be that we got ftrace_location() wrong in that
lookup_rec()'s end argument is inclusive, hence we need:

	lookup_rec(ip, ip + size - 1)

Now, the above thing asserts that:

	ftrace_location(x) == {0, x}

and that is genuinely false in my case, I get x+4 as additional possible
output. So I think I need the below change to go on top of all I have:


diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 7f0ce42f8ff9..4c13406e0bc4 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -198,13 +198,14 @@ __recover_probed_insn(kprobe_opcode_t *buf, unsigned long addr)
 
 	kp = get_kprobe((void *)addr);
 	faddr = ftrace_location(addr);
+
 	/*
-	 * Addresses inside the ftrace location are refused by
-	 * arch_check_ftrace_location(). Something went terribly wrong
-	 * if such an address is checked here.
+	 * In case addr maps to sym+0 ftrace_location() might return something
+	 * other than faddr. In that case consider it the same as !faddr.
 	 */
-	if (WARN_ON(faddr && faddr != addr))
-		return 0UL;
+	if (faddr && faddr != addr)
+		faddr = 0;
+
 	/*
 	 * Use the current code if it is not modified by Kprobe
 	 * and it cannot be modified by ftrace.