On Thu, Nov 13, 2014 at 11:22:10PM -0600, Josh Poimboeuf wrote:
> > The really trivial solution is:
> >
> >
> > static void lpc_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> > struct ftrace_ops *ops, struct pt_regs *regs)
> > {
> > struct lpc_func *func = ops->private;
> >
> > if (!within_patched_set(parent_ip))
> > regs->ip = func->new_addr;
> > }
> >
> >
> > Obviously, within_patched_set() would need an efficient data structure,
> > like an interval tree, to quickly decide whether the parent ip address
> > is within the patched set.
> >
> > This is enough to make sure that function calls are consistent.
> >
> > No stack checking, no stop_machine(), no forcing threads through
> > userspace.
> >
> > A disadvantage is that it doesn't give an indication when patching is
> > over and the trampoline can be removed. For that, I currently don't see
> > a better method than tracking kernel exits and/or stack analysis.
> >
> > What do you think?
>
> Interesting idea. But the hard part is figuring out when the patching
> is complete. Once you implement that, it might not be so minimal.
True.
The same applies to the current livepatch implementation, though. You
cannot tell if all threads have left the functions you've patched or
whether they're remaining on the stack and as such cannot remove patches
safely nor start transforming data.
The above implementation of extending the current livepatch to handle
function prototype changes can be further extended to handle multiple
patches 'in progress' by only jumping to a 'next level' patch if coming
from a function not changed by that level.
> BTW, here's a very simple way to implement LEAVE_PATCHED_SET +
> SWITCH_THREAD:
>
> for each thread {
> if (!on_cpu(thread) and !on_stack(thread, funcs) {
> switch thread to new universe
> } else if (unfreezable kthread) {
> retry and/or fail (kthread is either CPU-bound or is using patched func)
> } else {
> freeze thread
> if (!on_stack(thread, funcs))
> switch thread to new universe
> else
> fail (patch affects freezer/schedule path)
> }
> }
I think this could work. You need to make sure you can prevent a process
from waking up, however, to avoid race conditions in the first if().
A very similar method could be used to detect if the minimalistic
approach above finished patching, trampolines can be removed, data
patching started, etc.
> Note this approach will fail in a few cases, like if you have a
> CPU-bound kthread (but we can keep retrying in that case), if a patched
> function is in use by a sleeping kthread, or if the patch affects the
> freeze/schedule path.
None of the so far proposed methods can complete patching if a patched
function is in use by a sleeping kthread, or even any thread. So the
limitations you show aren't a huge problem.
> But the code is simple and converges to the patched state quickly.
>
> Thoughts?
You could avoid the freezing (and thus userspace impact) by tracking
syscall (or freezer) entry/exit like kGraft does. Any process cpu-bound
in userspace can be migrated immediately and any proces executing
syscalls can be migrated when on the kernel/userspace boundary. And
processes that are sleeping are already handled.
--
Vojtech Pavlik
Director SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
