Re: [RFC][Patch 2/2][take2]kprobe: kprobe-booster against 2.6.14-mm1 for i386

[Masami Hiramatsu <hiramatu at sdl dot hitachi dot co dot jp>]

Hi, Anil

Masami Hiramatsu wrote:
>>Issue No 1:
>>	In the above code patch, once you call reset_current_kprobes() and
>>preempt_enable_no_resched() ( I see preeempt_enable_no_resched()
>>in your fix to this code), you are not suppose to relay on
>>&p->ainsn.insn, the reason is that, on the other cpu technically
>>this memory can get freed due to unregister_kprobe() call.
>>In other words, by the time this cpu tries to execte the instruction
>>at regs->eip, that memory might have gone and system is bound to crash.
> 
> I think old djprobe has the same issue. So, it will be solved by using
> safety check routine after removing breakpoint. Current kprobe uses
> synchronize_sched() for waiting rcu processing in unregister_kprobe().
> In my opinion, the solution is that introducing safety check same as
> djprobe instead of synchronize_sched() in unregister_kprobe().

I found the rcu_barrier() function that works similar to djprobe's
safety check routine.

> The safety check routine of djprobe waits until all cpus execute the works,
> and the works are executed from the keventd.

The rcu_barrier() function waits until all cpus have gone through a
quiescent state.

- The CPU performs a process switch.
- The CPU starts executing in User Mode.
- The CPU executes the idle loop
In each of these cases, We say that the CPU has gone through
a quiescent state.

If we call rcu_barrier() after "int3" is removed, we can ensure
followings after the rcu_barrier() called.
- interrupt handlers are finished on all CPUs.
- p->ainsn.insn is not executed on all CPUs.
And we can remove a boosted kprobe safely.

Current kprobes uses synchronize_sched(). I think this function
checks safety of only current CPU. So, it may not ensure above
conditions. But rcu_barrier() checks safety of all CPUs. So, it
can ensure above conditions.
How would you think about this?

> And also, on preemptible kernel, the booster is not enabled where the
> kernel preemption is enabled. So, there are no preempted threads on
> p->ainsn.insn.


-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp