Skip to content

kprobes: improve error handling when arming/disarming kprobes

Jessica Yu lkml 5 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v5 Latest2 patches 5 repliesIn review
  2. v42 patches 1 replySuperseded
  3. v32 patches 3 repliesSuperseded
Earlier versions · 2
VersionPostedPatchesRepliesStatus
v22 patches8 repliesSuperseded
v12 patches5 repliesSuperseded

Changes in v5 from the cover letter

- Switch from WARN() to pr_debug() in arm_kprobe_ftrace() so the stack dumps don't pollute dmesg, as IPMODIFY conflicts can occur in normal usage
- Added Masami's ack to the first patch

Patches in v5 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2kprobes: propagate error from arm_kprobe_ftrace()kernel/kprobes.c+75 -250 replies
2/2kprobes: propagate error from disarm_kprobe_ftrace()kernel/kprobes.c+53 -250 replies

Reviews trailers per patch

All 2 patches have a Reviewed-by or Acked-by.

PatchPetr MladekMasami Hiramatsu
1/2Based-on-patches-by A-b
2/2Based-on-patches-by A-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v5 thread

Cover letter [PATCH v5 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@/home/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, and optimized kprobes. It passes the kprobes smoke test, but more testing is greatly appreciated.

Changes from v4:
- Switch from WARN() to pr_debug() in arm_kprobe_ftrace() so the stack dumps don't pollute dmesg, as IPMODIFY conflicts can occur in normal usage
- Added Masami's ack to the first patch

Changes from v3:
- Have (dis)arm_kprobe_ftrace() return -ENODEV instead of 0 in case of !CONFIG_KPROBES_ON_FTRACE
- Add total count of all probes tried in (dis)arm_all_kprobes()

Changes from v2:
- Add missing synchronize rcu in register_aggr_kprobe()
- s/kprobes/probes/ on error message in (dis)arm_all_kprobes()

Changes from v1:
- Don't arm the kprobe before adding it to the kprobe table, otherwise we'll temporarily see a stray breakpoint.
- Remove kprobe from the kprobe_table and call synchronize_sched() if arming during register_kprobe() fails.
- add Masami's ack on the 2nd patch (unchanged from v1)

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

 kernel/kprobes.c | 178 +++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 128 insertions(+), 50 deletions(-)
Signature · 1 line
2.13.6