From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753901AbdBNQkE (ORCPT ); Tue, 14 Feb 2017 11:40:04 -0500 Received: from smarthost03d.mail.zen.net.uk ([212.23.1.23]:54182 "EHLO smarthost03d.mail.zen.net.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750767AbdBNQj7 (ORCPT ); Tue, 14 Feb 2017 11:39:59 -0500 Message-ID: <1487090390.2978.12.camel@linaro.org> Subject: Re: [BUGFIX PATCH 3/3] kprobes/arm: Fix the return address of multiple kretprobes From: "Jon Medhurst (Tixy)" To: Masami Hiramatsu Cc: Russell King , linux-kernel@vger.kernel.org, Peter Zijlstra , Ananth N Mavinakayanahalli , Ingo Molnar , Thomas Gleixner , "H . Peter Anvin" , Wang Nan , Catalin Marinas , Will Deacon , "David A . Long" , Sandeepa Prabhu Date: Tue, 14 Feb 2017 16:39:50 +0000 In-Reply-To: <20170215010107.8aff7cb6f526502daf094879@kernel.org> References: <148699813647.8505.3671818093118978038.stgit@devbox> <148699834879.8505.2595570331866285215.stgit@devbox> <1487068371.2978.6.camel@linaro.org> <1487080027.2978.8.camel@linaro.org> <20170215010107.8aff7cb6f526502daf094879@kernel.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.4-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Originating-smarthost03d-IP: [82.69.122.217] Feedback-ID: 82.69.122.217 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-02-15 at 01:01 +0900, Masami Hiramatsu wrote: > On Tue, 14 Feb 2017 13:47:07 +0000 > "Jon Medhurst (Tixy)" wrote: > > > On Tue, 2017-02-14 at 10:32 +0000, Jon Medhurst (Tixy) wrote: > > > On Tue, 2017-02-14 at 00:05 +0900, Masami Hiramatsu wrote: > > > > This is arm port of commit 737480a0d525 ("kprobes/x86: > > > > Fix the return address of multiple kretprobes"). > > > > > > > > Fix the return address of subsequent kretprobes when multiple > > > > kretprobes are set on the same function. > > > > > > > > For example: > > > > > > > > # cd /sys/kernel/debug/tracing > > > > # echo "r:event1 sys_symlink" > kprobe_events > > > > # echo "r:event2 sys_symlink" >> kprobe_events > > > > # echo 1 > events/kprobes/enable > > > > # ln -s /tmp/foo /tmp/bar > > > > > > > > (without this patch) > > > > > > > > # cat trace | grep -v ^# > > > > ln-82 [000] dn.2 68.446525: event1: (kretprobe_trampoline+0x0/0x18 <- SyS_symlink) > > > > ln-82 [000] dn.2 68.447831: event2: (ret_fast_syscall+0x0/0x1c <- SyS_symlink) > > > > > > > > (with this patch) > > > > > > > > # cat trace | grep -v ^# > > > > ln-81 [000] dn.1 39.463469: event1: (ret_fast_syscall+0x0/0x1c <- SyS_symlink) > > > > ln-81 [000] dn.1 39.464701: event2: (ret_fast_syscall+0x0/0x1c <- SyS_symlink) > > > > > > > > Signed-off-by: Masami Hiramatsu > > > > Cc: KUMANO Syuhei > > > > --- > > > > > > I don't fully understand this function, but I've checked that the ARM > > > version now matches the x86 version (apart from the x86 specific > > > register fixup and some comments). So, FWIW > > > > > > Acked-by: Jon Medhurst > > > > > > I ran the before and after test case in the commit log on ARM and > > > verified the result is correct. I also tried running the ARM kprobe > > > tests with these 3 fixes but the tests fail. However, they also fail > > > without any of these changes, so I'll investigate that further... > > > > Bisecting the issue led me back to Linux 4.5 and commit 25362dc496ed > > ("ARM: 8501/1: mm: flip priority of CONFIG_DEBUG_RODATA") > > > > This sets CONFIG_DEBUG_RODATA to be enabled by default. If I disable > > that on 4.10-rc4, with the three patches in this series, then the ARM > > kprobes tests pass OK. > > > > I'll stick the DEBUG_RODATA issue on my todo list (it's been around for > > a year, so can probably wait a little longer). > > Hmm, I'm running arm kernel on qemu, which maybe the reason why > the test case passed in my environment, since my kconfig also sets > CONFIG_DEBUG_RODATA=y. > > BTW, would you see that any kprobe_events didn't work with > CONFIG_DEBUG_RODATA=y? (what the failure messages were?) The tests I'm running are the ARM specific tests that are enabled by CONFIG_ARM_KPROBES_TEST=y. I'm running the tests on real multicore ARM hardware (Versatile Express with a TC2 CoreTile) For me, sometimes the first test gave: Beginning kprobe tests... Probe ARM code     kprobe FAIL: test regs not OK Other times, for the specific instruction emulation tests they return FAIL: test_before_handler not run Not sure how much of the diagnostic appear without setting the tests to be verbose, which I do with: sed -e 's/VERBOSE 0/VERBOSE 1/' -i arch/arm/probes/kprobes/test-core.h Whilst writing a reply, I looked at the test code in arch/arm/probes/kprobes/test-core.c (which I wrote some years ago) and there is possibly a clue staring at us in the comments at the top of the file...  *  * The above would expand to assembler looking something like:  *  * @ TESTCASE_START  * bl __kprobes_test_case_start  * .pushsection .rodata  * "10:  * .ascii "mov r0, r7" @ text title for test case  * .byte 0  * .popsection  * @ start of inline data...  * .word 10b @ pointer to title in .rodata section Note the ".pushsection .rodata" (though I don't see an immediate obvious reason why that would cause a problem. It certainly seems likely that the problem is with the ARM test code rather than actual kprobe implementation itself. Like I said, this issue has been there for a year or more, so I wasn't planning on spending time on it for a few more days yet whilst I get on with other urgent matters. -- Tixy Basically, m