linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@elte.hu>, Anton Arapov <aarapov@redhat.com>,
	David Smith <dsmith@redhat.com>,
	"Frank Ch. Eigler" <fche@redhat.com>,
	Martin Cermak <mcermak@redhat.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] uprobes: Change uprobe_copy_process() to dup return_instances
Date: Mon, 14 Oct 2013 21:00:54 +0200	[thread overview]
Message-ID: <20131014190054.GA15710@redhat.com> (raw)
In-Reply-To: <20131014184548.GG2675@laptop.programming.kicks-ass.net>

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.


  reply	other threads:[~2013-10-14 19:08 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-13 19:18 [PATCH 0/5] uprobes: fix fork() with the pending ret-probe(s) Oleg Nesterov
2013-10-13 19:18 ` [PATCH 1/5] uprobes: Change the callsite of uprobe_copy_process() Oleg Nesterov
2013-10-16 12:37   ` Srikar Dronamraju
2013-10-13 19:18 ` [PATCH 2/5] uprobes: Introduce __create_xol_area() Oleg Nesterov
2013-10-16 12:41   ` Srikar Dronamraju
2013-10-16 12:50     ` Srikar Dronamraju
2013-10-13 19:18 ` [PATCH 3/5] uprobes: Teach __create_xol_area() to accept the predefined vaddr Oleg Nesterov
2013-10-16 12:43   ` Srikar Dronamraju
2013-10-13 19:18 ` [PATCH 4/5] uprobes: Change uprobe_copy_process() to dup return_instances Oleg Nesterov
2013-10-14 18:45   ` Peter Zijlstra
2013-10-14 19:00     ` Oleg Nesterov [this message]
2013-10-16 12:47   ` Srikar Dronamraju
2013-10-13 19:18 ` [PATCH 5/5] uprobes: Change uprobe_copy_process() to dup xol_area Oleg Nesterov
2013-10-14 14:09   ` Peter Zijlstra
2013-10-14 14:55     ` Oleg Nesterov
2013-10-14 15:47       ` Peter Zijlstra
2013-10-16 12:53   ` Srikar Dronamraju
2013-10-16 16:09     ` Oleg Nesterov
2013-10-18 15:49   ` Oleg Nesterov
2013-10-14 18:29 ` [PATCH 0/5] uprobes: fix fork() with the pending ret-probe(s) Oleg Nesterov
2013-10-16 17:38   ` Oleg Nesterov
2013-10-16 17:39 ` [PATCH 6/5] uprobes: Teach uprobe_copy_process() to handle CLONE_VFORK Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131014190054.GA15710@redhat.com \
    --to=oleg@redhat.com \
    --cc=aarapov@redhat.com \
    --cc=dsmith@redhat.com \
    --cc=fche@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcermak@redhat.com \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=srikar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).