Re: [PATCH V3 3/4] arch/powerpc: Implement Optprobes

[Masami Hiramatsu]

On Sat, 4 Feb 2017 01:09:49 +0530
"Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> wrote:

> Hi Michael,
> Thanks for the review! I'll defer to Anju on most of the aspects, but...
> 
> On 2017/02/01 09:53PM, Michael Ellerman wrote:
> > Anju T Sudhakar <anju@xxxxxxxxxxxxxxxxxx> writes:
> > 
> > > +static void optimized_callback(struct optimized_kprobe *op,
> > > +			       struct pt_regs *regs)
> > > +{
> > > +	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> > > +	unsigned long flags;
> > > +
> > > +	/* This is possible if op is under delayed unoptimizing */
> > > +	if (kprobe_disabled(&op->kp))
> > > +		return;
> > > +
> > > +	local_irq_save(flags);
> > 
> > What is that protecting against? Because on powerpc it doesn't actually
> > disable interrupts, it just masks some of them, the perf interrupt for
> > example can still run.
> 
> That's an excellent catch, as always! :)
> 
> This is meant to prevent us from missing kprobe hits while processing 
> interrupts that arrive when this optprobe is being handled. And you are 
> totally right -- we would miss kprobe hits during PMI handling with the 
> current approach. We need a hard_irq_disable() there.

One note: it depends on the arch implementation of kprobes, since this
is only for "emulating" the int3 behavior on x86 for compatibility.
On x86, int3 is disabling interrupt automatically, so all the kprobes
user handlers will be run under irq-disabled. This means that user may
write their code to run as such condition. They even can not know
that is optimized or not at programming timing, because the kprobe
will be optimized after a while from enabled it.

So the important point is that you have to keep it compatible of
unoptimized kprobes.

Thank you,

-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>