Commit 087eb437051b3de817720f9c80c440fc9e7dcce8

Authored by Oleg Nesterov
Committed by Linus Torvalds
1 parent b87297fb40

ptrace: tracehook_report_clone: fix false positives

The "trace || CLONE_PTRACE" check in tracehook_report_clone() is not right,

- If the untraced task does clone(CLONE_PTRACE) the new child is not traced,
  we must not queue SIGSTOP.

- If we forked the traced task, but the tracer exits and untraces both the
  forking task and the new child (after copy_process() drops tasklist_lock),
  we should not queue SIGSTOP too.

Change the code to check task_ptrace() != 0 instead. This is still racy, but
the race is harmless.

We can race with another tracer attaching to this child, or the tracer can
exit and detach in parallel. But giwen that we didn't do wake_up_new_task()
yet, the child must have the pending SIGSTOP anyway.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Roland McGrath <roland@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Showing 2 changed files with 6 additions and 7 deletions Side-by-side Diff

include/linux/tracehook.h
... ... @@ -259,14 +259,12 @@
259 259  
260 260 /**
261 261 * tracehook_report_clone - in parent, new child is about to start running
262   - * @trace: return value from tracehook_prepare_clone()
263 262 * @regs: parent's user register state
264 263 * @clone_flags: flags from parent's system call
265 264 * @pid: new child's PID in the parent's namespace
266 265 * @child: new child task
267 266 *
268   - * Called after a child is set up, but before it has been started
269   - * running. @trace is the value returned by tracehook_prepare_clone().
  267 + * Called after a child is set up, but before it has been started running.
270 268 * This is not a good place to block, because the child has not started
271 269 * yet. Suspend the child here if desired, and then block in
272 270 * tracehook_report_clone_complete(). This must prevent the child from
273 271  
274 272  
... ... @@ -276,13 +274,14 @@
276 274 *
277 275 * Called with no locks held, but the child cannot run until this returns.
278 276 */
279   -static inline void tracehook_report_clone(int trace, struct pt_regs *regs,
  277 +static inline void tracehook_report_clone(struct pt_regs *regs,
280 278 unsigned long clone_flags,
281 279 pid_t pid, struct task_struct *child)
282 280 {
283   - if (unlikely(trace) || unlikely(clone_flags & CLONE_PTRACE)) {
  281 + if (unlikely(task_ptrace(child))) {
284 282 /*
285   - * The child starts up with an immediate SIGSTOP.
  283 + * It doesn't matter who attached/attaching to this
  284 + * task, the pending SIGSTOP is right in any case.
286 285 */
287 286 sigaddset(&child->pending.signal, SIGSTOP);
288 287 set_tsk_thread_flag(child, TIF_SIGPENDING);
... ... @@ -1409,7 +1409,7 @@
1409 1409 }
1410 1410  
1411 1411 audit_finish_fork(p);
1412   - tracehook_report_clone(trace, regs, clone_flags, nr, p);
  1412 + tracehook_report_clone(regs, clone_flags, nr, p);
1413 1413  
1414 1414 /*
1415 1415 * We set PF_STARTING at creation in case tracing wants to