From: Oleg Nesterov <oleg@redhat•com>
To: Vladimir Zapolskiy <vzapolskiy@gmail•com>
Cc: Evgeniy Polyakov <zbr@ioremap•net>,
"David S. Miller" <davem@davemloft•net>,
Roland McGrath <roland@redhat•com>,
netdev@vger•kernel.org
Subject: Re: [PATCH] connector: add an event for monitoring process tracers
Date: Wed, 13 Jul 2011 17:09:42 +0200 [thread overview]
Message-ID: <20110713150942.GA4850@redhat.com> (raw)
In-Reply-To: <1310502757-32103-1-git-send-email-vzapolskiy@gmail.com>
On 07/12, Vladimir Zapolskiy wrote:
>
> Note, a detach signal is not emitted, if a tracer process terminates
> without explicit PTRACE_DETACH request. Such cases can be covered
> listening to PROC_EVENT_EXIT connector events.
Hmm. More and more reasons to make the implicit detach sleepable...
But. There is another case. The (dead) tracee can be detached via
do_wait(). Perhaps this falls into "covered listening to EXIT" too,
but imho makes sense to document in the changelog. Oh, and probably
we will add the ability to detach a zombie...
I don't really understand why do you need this, but I won't argue.
As for the patch,
> +void proc_ptrace_connector(struct task_struct *task)
> +{
> + struct cn_msg *msg;
> + struct proc_event *ev;
> + struct timespec ts;
> + __u8 buffer[CN_PROC_MSG_SIZE];
> + struct task_struct *tracer;
> +
> + if (atomic_read(&proc_event_num_listeners) < 1)
> + return;
> +
> + msg = (struct cn_msg *)buffer;
> + ev = (struct proc_event *)msg->data;
> + get_seq(&msg->seq, &ev->cpu);
> + ktime_get_ts(&ts); /* get high res monotonic timestamp */
> + put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
> + ev->what = PROC_EVENT_PTRACE;
> + ev->event_data.ptrace.process_pid = task->pid;
> + ev->event_data.ptrace.process_tgid = task->tgid;
> +
> + rcu_read_lock();
> + tracer = tracehook_tracer_task(task);
> + if (tracer) {
> + ev->event_data.ptrace.tracer_pid = tracer->pid;
> + ev->event_data.ptrace.tracer_tgid = tracer->tgid;
> + } else {
> + ev->event_data.ptrace.tracer_pid = 0;
> + ev->event_data.ptrace.tracer_tgid = 0;
> + }
This doesn't look right. The code uses tracehook_tracer_task() to
figure out whether this task traced or not. But this is racy.
ptrace_attach:
...attach...
/* WINDOW */
proc_ptrace_connector(task);
The task can exit in between, and the caller's subthread can do
wait4() and release it. In this case proc_ptrace_connector() will
see tracehook_tracer_task() == NULL and report "detach".
The similar race in ptrace_detach() path. Another tracer can attach
to this task before we proc_ptrace_connector().
I think proc_ptrace_connector() needs the explicit "task_struct *tracer"
argument, NULL if ptrace_detach(). Or a simple boolean, the tracer is
current.
If you think this is fine - I won't argue.
But in any case, please rediff against
git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc.git ptrace
tracehook_tracer_task() was removed, and
> @@ -260,6 +261,9 @@ out:
> if (wait_trap)
> wait_event(current->signal->wait_chldexit,
> !(task->group_stop & GROUP_STOP_TRAPPING));
> + if (!retval)
> + proc_ptrace_connector(task);
> +
> return retval;
> }
this chunk probably should be updated.
Oleg.
next prev parent reply other threads:[~2011-07-13 15:12 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-12 20:32 [PATCH] connector: add an event for monitoring process tracers Vladimir Zapolskiy
2011-07-13 12:48 ` Evgeniy Polyakov
2011-07-13 14:53 ` David Miller
2011-07-13 15:09 ` Oleg Nesterov [this message]
2011-07-15 17:41 ` Vladimir Zapolskiy
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=20110713150942.GA4850@redhat.com \
--to=oleg@redhat$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=roland@redhat$(echo .)com \
--cc=vzapolskiy@gmail$(echo .)com \
--cc=zbr@ioremap$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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