linux-kernel - [PATCH 0/2] kprobes: improve error handling when arming/disarming kprobes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20171004191414.8872-1-jeyu@kernel.org>
Date:   Wed,  4 Oct 2017 21:14:12 +0200
From:   Jessica Yu <jeyu@...nel.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>,
        Petr Mladek <pmladek@...e.com>,
        Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S . Miller" <davem@...emloft.net>,
        Ingo Molnar <mingo@...nel.org>
Cc:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Jiri Kosina <jikos@...nel.org>,
        Miroslav Benes <mbenes@...e.cz>,
        Steven Rostedt <rostedt@...dmis.org>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jessica Yu <jeyu@...nel.org>
Subject: [PATCH 0/2] kprobes: improve error handling when arming/disarming kprobes

Hi,

This patchset attempts to improve error handling when arming or disarming
ftrace-based kprobes. The current behavior is to simply WARN when ftrace
(un-)registration fails, without propagating the error code. This can lead
to confusing situations where, for example, register_kprobe()/enable_kprobe()
would return 0 indicating success even if arming via ftrace had failed. In
this scenario we'd end up with a non-functioning kprobe even though kprobe
registration (or enablement) returned success. In this patchset, we take
errors from ftrace into account and propagate the error when we cannot arm
or disarm a kprobe.

Below is an example that illustrates the problem using livepatch and
systemtap (which uses kprobes underneath). Both livepatch and kprobes use
ftrace ops with the IPMODIFY flag set, so registration at the same
function entry is limited to only one ftrace user. 

Before
------
# modprobe livepatch-sample 	# patches cmdline_proc_show, ftrace ops has IPMODIFY set
# stap -e 'probe kernel.function("cmdline_proc_show").call { printf ("cmdline_proc_show\n"); }'

   .. (nothing prints after reading /proc/cmdline) ..

The systemtap handler doesn't execute due to a kprobe arming failure caused
by a ftrace IPMODIFY conflict with livepatch, and there isn't an obvious
indication of error from systemtap (because register_kprobe() returned
success) unless the user inspects dmesg.

After
-----
# modprobe livepatch-sample 
# stap -e 'probe kernel.function("cmdline_proc_show").call { printf ("cmdline_proc_show\n"); }'
WARNING: probe kernel.function("cmdline_proc_show@...me/jeyu/work/linux-next/fs/proc/cmdline.c:6").call (address 0xffffffffa82fe910) registration error (rc -16)

Although the systemtap handler doesn't execute (as it shouldn't), the
ftrace error is propagated and now systemtap prints a visible error message
stating that (kprobe) registration had failed (because register_kprobe()
returned an error), along with the propagated error code.

This patchset was based on Petr Mladek's original patchset (patches 2 and 3)
back in 2015, which improved kprobes error handling, found here:

   https://lkml.org/lkml/2015/2/26/452

However, further work on this had been paused since then and the patches
were not upstreamed.

This patchset has been lightly sanity-tested (on linux-next) with kprobes,
kretprobes, jprobes, and optimized kprobes. It passes the kprobes smoke
test, but more testing is greatly appreciated.

Thanks,

Jessica

---
Jessica Yu (2):
  kprobes: propagate error from arm_kprobe_ftrace()
  kprobes: propagate error from disarm_kprobe_ftrace()

 kernel/kprobes.c | 163 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 112 insertions(+), 51 deletions(-)

-- 
2.13.6