linux-kernel - [PATCH 1/3] ptrace_detach: the wrong wakeup breaks the ERESTARTxxx logic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 8 Feb 2009 19:47:24 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Jerome Marchand <jmarchan@...hat.com>,
	Roland McGrath <roland@...hat.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: [PATCH 1/3] ptrace_detach: the wrong wakeup breaks the ERESTARTxxx
	logic

Another ancient bug. Consider this trivial test-case,

	int main(void)
	{
		int pid = fork();

		if (pid) {
			ptrace(PTRACE_ATTACH, pid, NULL, NULL);
			wait(NULL);
			ptrace(PTRACE_DETACH, pid, NULL, NULL);
		} else {
			pause();
			printf("WE HAVE A KERNEL BUG!!!\n");
		}

		return 0;
	}

the child must not "escape" for sys_pause(), but it can and this was seen
in practice.

This is because ptrace_detach does:

	if (!child->exit_state)
		wake_up_process(child);

this wakeup can happen after this child has already restarted sys_pause(),
because it gets another wakeup from ptrace_untrace().

It is not clear to me what was the rationale, but this is obviously wrong.
Remove this wakeup. The caller saw this task in TASK_TRACED state, and
unless it was SIGKILL'ed in between __ptrace_unlink()->ptrace_untrace()
should handle this case correctly. If it was SIGKILL'ed, we don't need
to wakup the dying tracee too.

Signed-off-by: Oleg Nesterov <oleg@...hat.com>

--- 6.29-rc3/kernel/ptrace.c~1_KILL_WAKE	2009-02-08 04:26:57.000000000 +0100
+++ 6.29-rc3/kernel/ptrace.c	2009-02-08 05:11:52.000000000 +0100
@@ -250,11 +250,7 @@ int ptrace_detach(struct task_struct *ch
 	/* protect against de_thread()->release_task() */
 	if (child->ptrace) {
 		child->exit_code = data;
-
 		dead = __ptrace_detach(current, child);
-
-		if (!child->exit_state)
-			wake_up_process(child);
 	}
 	write_unlock_irq(&tasklist_lock);

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