Re: [PATCH 4/5] uprobes: Change uprobe_copy_process() to dupreturn_instances

[Oleg Nesterov]

On 10/14, Peter Zijlstra wrote:
>
> On Sun, Oct 13, 2013 at 09:18:41PM +0200, Oleg Nesterov wrote:
> > uprobe_copy_process() assumes that the new child doesn't need
> > ->utask, it should be allocated by demand.
> >
> > But this is not true if the forking task has the pending ret-
> > probes, the child should report them as well and thus it needs
> > the copy of parent's ->return_instances chain. Otherwise the
> > child crashes when it returns from the probed function.
>
> So children don't automagically inherit the same probes

They actually do. And in this case they also "inherit" the fact that
the probed function was called, even if the forked child didn't do
this actually.

> so wouldn't simply fixing up the
> child stack be a solution?

This was plan A ;)

> If not; its not entirely clear to my why this isn't a good solution

Tthis doesn't look correct, although "correct" is subjective and we
never tried to enforce the rules before. But at least stap wants to
see the reports from the child.

Another reason is that this needs the arch-specific changes/hooks.
Say, I simply do not know how we can "revert" the effect of
"regs->link = trampoline_vaddr" on powerpc, this looks simply
impossible.

And even on x86 we either need __access_remote_vm() from copy_process()
or or dup_utask() + task_work_run() so that the child can do this itself.

(plus we also need to change prepare_uretprobe(), say, on x86 it should
 record regs->sp in return_instance, but this is minor).

> based on these changelogs.

Note the "the child should report them as well"... but yes, agreed,
I will update the changelog.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/