Comments (18)
Hi @liu-song-6 : thanks for checking, Masami fixed this in the kernel a while back with [PATCH] kprobes: Allow kprobes coexist with livepatch, so we can close this long-open issue.
from kpatch.
Recreate procedure:
First start systemtap to enable the kprobe:
~ $ sudo stap -v -e 'probe kernel.function("meminfo_proc_show") {printf("meminfo_proc_show called\n");}'
Pass 1: parsed user script and 104 library script(s) using 218480virt/35716res/3064shr/33300data kb, in 110usr/10sys/114real ms.
Pass 2: analyzed script: 1 probe(s), 0 function(s), 0 embed(s), 0 global(s) using 409876virt/173104res/105960shr/67656data kb, in 480usr/60sys/542real ms.
Pass 3: using cached /root/.systemtap/cache/9f/stap_9f022595f0a4c768e04c75362b24228b_975.c
Pass 4: using cached /root/.systemtap/cache/9f/stap_9f022595f0a4c768e04c75362b24228b_975.ko
Pass 5: starting run.
Then in another window:
~ $ cat meminfo-string.patch
Index: src/fs/proc/meminfo.c
===================================================================
--- src.orig/fs/proc/meminfo.c
+++ src/fs/proc/meminfo.c
@@ -95,7 +95,7 @@ static int meminfo_proc_show(struct seq_
"Committed_AS: %8lu kB\n"
"VmallocTotal: %8lu kB\n"
"VmallocUsed: %8lu kB\n"
- "VmallocChunk: %8lu kB\n"
+ "VMALLOCCHUNK: %8lu kB\n"
#ifdef CONFIG_MEMORY_FAILURE
"HardwareCorrupted: %5lu kB\n"
#endif
~ $ kpatch build meminfo-string.patch
Using cache at /home/jpoimboe/.kpatch/3.13.5-202.fc20.x86_64/src
Building original kernel
Building patched kernel
Detecting changed objects
Rebuilding changed objects
Extracting new and modified ELF sections
Building patch module: kpatch-meminfo-string.ko
SUCCESS
~ $ grep -i chunk /proc/meminfo
VmallocChunk: 34359330256 kB
~ $ sudo insmod kpatch-meminfo-string.ko
~ $ grep -i chunk /proc/meminfo
VmallocChunk: 34359330256 kB
Notice the patch doesn't work. Now kill the stap process and the patch takes effect:
~ $ grep -i chunk /proc/meminfo
VMALLOCCHUNK: 34359330256 kB
from kpatch.
I had a good discussion about this with Masami Hiramatsu (kprobes maintainer). In my opinion we should go with option 4 because I think kpatching a kprobed function is an obscure and confusing use case that isn't worth supporting. Option 4 requires a new kprobes interface, but it resolves multiple issues (3 by my count).
from kpatch.
Option 4 still isn't perfect though. You wouldn't be able to kpatch a kprobed function, but you could still kprobe a kpatched function, which could cause a little bit of confusion if the kprobes were on the original version of the function.
from kpatch.
Also, we must consider the probing case after patching. We should introduce FTRACE_OPS_FL_IPMODIFY and if there is already a handler which has the flag at the given address, ftrace should reject new one.
This makes the solution easier. Kprobes and kpatch don't need to consider each other directly, but just get an error when using ftrace.
from kpatch.
@mhiramat Yes, if some kprobes handlers modify IP, then I think an FTRACE_OPS_FL_IPMODIFY flag makes sense. One question: how will kprobes know if a given handler will modify the IP? Or would it always set the flag regardless?
@rostedt had also suggested a FTRACE_OPS_FL_PERMANENT flag, so that a patch wouldn't ever get removed, even if function_trace_stop gets set.
from kpatch.
@jpoimboe all kprobes handlers is possible to change IP and it also requires the original (unmodified) IP address since it gets a kprobe handler from that. So anyway all kprobes must set the FTRACE_OPS_FL_IPMODIFY. to ensure ftrace passes unmodified IP.
from kpatch.
@mhiramat I talked with Steven about this a little bit. The problem with kprobes always setting FTRACE_OPS_FL_IPMODIFY is that it makes kprobes and kpatch always incompatible. If kprobes doesn't change regs->ip (which is probably true 99% of the time) then they should be able to co-exist.
Another problem is that ftrace can't enforce it. If somebody doesn't provide FTRACE_OPS_FL_IPMODIFY, it would be hard for ftrace to verify that they aren't changing regs->ip.
So how about this:
- If a kprobe handler needs to modify IP, user sets KPROBE_FLAG_IPMODIFY flag to register_kprobe, and then kprobes sets FTRACE_OPS_FL_IPMODIFY when registering with ftrace for that probe.
- if KPROBE_FLAG_IPMODIFY is not used, kprobe_ftrace_handler() can detect when a kprobe handler changes regs->ip and restore it to its original value (regs->ip = ip).
from kpatch.
@jpoimboe I think your suggestion is OK for me. Most of the case, jprobe is the only kprobe user which can modify IP, and jprobe itself is rarely used.
from kpatch.
@mhiramathitachi Any chance you discussed this with Steven at LinuxCon?
from kpatch.
(2014/05/28 12:18), Josh Poimboeuf wrote:
@mhiramathitachi Any chance you discussed this with Steven at LinuxCon?
Ah, I've heard that Steven will not be there. Last week I met him at LinuxCon Japan
and talked about kpatch and others.
Anyway I'll fix this issue on upstream kernel asap, since only kprobes is the actual
user on current upstream kernel.
Thank you, :)
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
from kpatch.
Sorry, I meant to say LinuxCon Japan :-) Are you planning on doing something like KPROBE_FLAG_IPMODIFY and FTRACE_OPS_FL_IPMODIFY like described above?
from kpatch.
Ah, I see :)
Yes, I'll make a patch and send them to Steven!
from kpatch.
Housekeeping: is there anything left to do that the introduction of the FTRACE_OPS_FL_IPMODIFY
doesn't fix? That was added way back in Nov 2014 with f8b8be8a310a ftrace, kprobes: Support IPMODIFY flag to find IP modify conflict.
I still see kprobe_ftrace_ops() always setting FTRACE_OPS_FL_IPMODIFY
so maybe @jpoimboe's suggestions are still TODO?
from kpatch.
Wow this is an old one. @mhiramathitachi @mhiramat are you still there? :-)
The original suggestion was
- If a kprobe handler needs to modify IP, user sets KPROBE_FLAG_IPMODIFY flag to register_kprobe, and then kprobes sets FTRACE_OPS_FL_IPMODIFY when registering with ftrace for that probe.
- if KPROBE_FLAG_IPMODIFY is not used, kprobe_ftrace_handler() can detect when a kprobe handler changes regs->ip and restore it to its original value (regs->ip = ip).
I don't think this has been much of a problem. Masami, do you have any plans to do that? If not, we can close this.
from kpatch.
@jpoimboe, Thank you for correcting my account.:-)
Hmm, I have to look it carefully, since I and bpf guys already introduced generic fault-injection using kprobes.
from kpatch.
Hi folks,
Is this still a problem? It works fine in my tests. My test steps are:
- Use bpftrace to start a ftrace-kprobe;
- Load live patch for the same function;
- Verify the bpftrace still works;
- Verify the function is patched (based on behavior).
from kpatch.
- Use bpftrace to start a ftrace-kprobe;
- Load live patch for the same function;
- Verify the bpftrace still works;
- Verify the function is patched (based on behavior).
Also, when I unload the live patch in 2, the bpftrace in 1 continues to work as expected.
from kpatch.
Related Issues (20)
- Do we need more robust archeticture protection HOT 4
- kpatch-build: verify_patch_files might miss a parameter HOT 2
- ERROR in find_local_syms, couldn't find matching XXX local symbols in vmlinux symbol table HOT 14
- Can you add support for Rocky and Alma? HOT 1
- relocation with type R_X86_64_GOTPCREL is not supported HOT 5
- Arch Compile kpatch-git test failed HOT 4
- Regarding "statically allocated data" again HOT 2
- kpatch-build error when modifying an object file's only syscall
- nowhere to find the definition of klp_register_patch HOT 5
- x86 paravirt code uses alternatives v6.8+ HOT 1
- special-static.patch can fail to build on s390x / upstream v6.8 HOT 6
- CONFIG_FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY not supported on ppc64le HOT 6
- function __pfx_new_sync_read has no fentry/mcount call, unable to patch HOT 6
- Unchanged and unpatchable function moves from .text to .text.unlikely HOT 5
- Why did the kpatch-build script export the KPATCH-BUILD environment variable from the BUILD directory to the SOURCE directory? HOT 3
- CONFIG_WERROR=y and CONFIG_LD_ORPHAN_WARN_LEVEL="error" break kpatch-build HOT 2
- linux 6.9.0-rc6 can't find special struct paravirt_patch_site size HOT 6
- fair.o: changed section .data..read_mostly not selected for inclusion HOT 11
- find_local_syms: 218: couldn't find matching cm.c local symbols in ib_mad symbol table HOT 7
- Kernel Warning: Unpatched return thunk in use. This should not happen! HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from kpatch.