Comments (7)
@mhiramathitachi Thanks, this is a very interesting approach.
My biggest concern is that this will increase the chances of the safety check failing and returning EBUSY. In many (or even most) cases, patches will be applied by some tools automatically, and we really want to avoid having to try to tell the user that a patch failed to apply. I suspect that minimizing the chance of insmod failure is more important than avoiding stop_machine latencies.
What about the accuracy of the stack walk logic while the other CPUs are running processes? Is it safe to walk the stack of a running process that is changing the stack simultaneously?
What happens if the function never returns and the timer expires? Do we just force the counter to 0? If so, that could create complications if the function happens to return later during a future update.
Another concern is that it relies on CONFIG_KPROBES and CONFIG_KRETPROBES, which could be a problem if users want a more minimally configured kernel for some reason. Though this might be a minor point.
The fact that kretprobes aren't yet NMI safe may be a hindrance for merging something like this soon, since e.g. we'd lose support for the Fedora 20 kernel (but obviously that's just a short term problem).
from kpatch.
(2014/04/24 22:28), Josh Poimboeuf wrote:
My biggest concern is that this will increase the chances of the safety check failing and returning EBUSY.
I don't think so, this approach may increase the chance of applying patch, since it checks
every timing of exiting the target functions a while. Anyway, the failing chance will be
increased if we patch more functions at once. So I think incremental (separated) patch
should be discussed.
In many (or even most) cases, patches will be applied by some tools automatically, and we really want to avoid having to try to tell the user that a patch failed to apply. I suspect that minimizing the chance of insmod failure is more important than avoiding stop_machine latencies.
It depends on user demand, and I think some tools can handle safety check failure and
retry it. Also, we can support both stop_machine-less and stop_machine.
What about the accuracy of the stack walk logic while the other CPUs are running processes? Is it safe to walk the stack of a running process that is changing the stack simultaneously?
As far as taking a task_lock, the stack will be never freed and safe to walk.
Even if the process goes into the target functions, it is automatically counted in the ftrace
handler.
What happens if the function never returns and the timer expires? Do we just force the counter to 0? If so, that could create complications if the function happens to return later during a future update.
At that time, we'll remove (unregister) kretprobes, then its handler is just skipped. :)
Another concern is that it relies on CONFIG_KPROBES and CONFIG_KRETPROBES, which could be a problem if users want a more minimally configured kernel for some reason. Though this might be a minor point.
Without kretprobes, we have to use stop_machine. But I guess that such user may disable
CONFIG_FTRACE first...
The fact that kretprobes aren't yet NMI safe may be a hindrance for merging something like this soon, since e.g. we'd lose support for the Fedora 20 kernel (but obviously that's just a short term problem).
Yes, I think this will take a time. There are pros and cons on this method, and we have
a time to discuss it. :)
from kpatch.
Sorry, accidental close :-)
from kpatch.
On Sun, Apr 27, 2014 at 07:24:34AM -0700, Masami Hiramatsu (Hitachi) wrote:
(2014/04/24 22:28), Josh Poimboeuf wrote:
My biggest concern is that this will increase the chances of the safety check failing and returning EBUSY.
I don't think so, this approach may increase the chance of applying patch, since it checks
every timing of exiting the target functions a while. Anyway, the failing chance will be
increased if we patch more functions at once. So I think incremental (separated) patch
should be discussed.
Well, for example, at least functions (and their subfunctions) which
never sleep, or which run with preemption disabled, will be more likely
to fail, since they could never even be on the stack during
stop_machine.
Also it will be somewhat less deterministic, which makes testing less
reliable for users deploying to lots of systems.
Incremental patching is a useful feature, but it's inherently more
dangerous, because each patch is built against the kernel, not against
the other patches. I talked about this in #81.
In many (or even most) cases, patches will be applied by some tools automatically, and we really want to avoid having to try to tell the user that a patch failed to apply. I suspect that minimizing the chance of insmod failure is more important than avoiding stop_machine latencies.
It depends on user demand, and I think some tools can handle safety check failure and
retry it.
Yeah, it's difficult to say without hearing more from real users.
Another idea to avoid safety failures is that if we detect an in-use
function, we can set an ftrace handler for the function, wake it up, and
then retry the patching procedure. Though I'm not sure whether it would
be a good idea.
Also, we can support both stop_machine-less and stop_machine.
This is possible, but to me it seems like a lot of complexity for not
much gain. What exactly are the problems with stop_machine we're trying
to solve? Latency? -EBUSY failures?
It's hard to gauge the importance of these limitations without having
more users to complain about them :-) But as one of the few current
users of kpatch, they haven't affected me yet.
What about the accuracy of the stack walk logic while the other CPUs are running processes? Is it safe to walk the stack of a running process that is changing the stack simultaneously?
As far as taking a task_lock, the stack will be never freed and safe to walk.
Even if the process goes into the target functions, it is automatically counted in the ftrace
handler.
But even with a task_lock, the task can can change the stack while we're
attempting to read it. We could see inconsistencies or corruption.
What happens if the function never returns and the timer expires? Do we just force the counter to 0? If so, that could create complications if the function happens to return later during a future update.
At that time, we'll remove (unregister) kretprobes, then its handler is just skipped. :)
Hm, sounds racy to me. More details would help :-)
from kpatch.
But even with a task_lock, the task can can change the stack while we're
attempting to read it. We could see inconsistencies or corruption.
To solve this problem, I've decided to use a running task list and check those tasks in a hook of context switching. :) In this way, all sleeping tasks can be checked safely and other running tasks are listed. After that hooking the context switch and wait for all running tasks are switched. Since all running tasks are checked in its context, it should be safely done.
from kpatch.
Here, I've pushed patches on my tree.
https://github.com/mhiramathitachi/kpatch/tree/no-stopmachine-v1
Note that this is just a proof of concept, not for upstream. We still need some enhancements on upstream kernel (at least IPMODIFY series required). But anyway, you can see how it works :)
from kpatch.
I think we can close this issue. kpatch.ko can continue to use stop_machine until it retires (when livepatch closes the feature gap).
from kpatch.
Related Issues (20)
- kpatch module build could fail when kernel 5.19+ contains dynamic symbols HOT 21
- kpatch_version: Unknown symbol kpatch_unregister HOT 8
- '-'/'_' mismatch for OOT modules HOT 2
- kpatch load can't find core module HOT 8
- Fix possible buffer overflow in ESP transformation error System.map error HOT 3
- integration tests: CONFIG_MODVERSIONS and linux-5.18.0/symvers-disagreement-FAIL.patch HOT 8
- After the kpatch install "xxx. ko" on openeuler, the machine was restarted. The kpatch service failed, resulting in the kernel patch restart not taking effect
- What do I need to do to support riscv64? HOT 11
- ERROR: testmod_drv.o: .return_sites section header details differ from .return_sites HOT 20
- Need suggestions for CONFIG_LTO_CLANG support HOT 40
- New error building patch for 6.2-rc2? HOT 12
- kpatch-build errors for kernel v6.1.4 HOT 4
- ubsan for kpatch HOT 2
- find_local_syms for <modified-file>: found_none HOT 2
- 6.3 failure with setlocalversion HOT 5
- Outstanding clang issues for 6.2 HOT 3
- x86 NOP padded functions without __pfx_ symbol HOT 4
- Linux 6.1 LTS: livepatch module fails to load HOT 26
- create-diff-object static local variable correlation and inlining HOT 5
- 1.0 release HOT 2
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.