Q: utrace && ptrace_check_attach()

[Oleg Nesterov]

On 07/22, Roland McGrath wrote:
>
> > This means ptrace_check_attach() can succeed but the tracee can even return
> > to the user-space after that, no? There is no guarantee utrace_get_signal()
> > will call finish_resume_report(). For example, if there are no pending
> > signals, we just clear ->stopped and return after dequeue_signal().
>
> Yes, I think you're right.  I think the "if (unlikely(utrace->stopped)) {"
> case in utrace_get_signal() needs to do some variant of utrace_reset.  If
> any engine_wants_stop() then it should not dequeue a signal even if one is
> ready, and probably it should also not do any spontaneous report.

Hmm. If ->stopped == T after return from do_signal_stop()->schedule(),
then we must have the engine which is engine_wants_stop(), no?

> OTOH perhaps it should do a spontaneous report in that case, so that if you
> used utrace_control(,,UTRACE_STOP) while in TASK_STOPPED, you get a
> UTRACE_SIGNAL_REPORT callback at SIGCONT time.  If that's the plan, then
> all we need is utrace_do_stop()'s task_is_stopped() case to set ->report
> too (and set_notify_resume for the invariant, though it's superfluous really).

Damn. Just can't think today...

Roland, can we just remove all code which plays with ->stopped in
utrace_report_jctl() and utrace_get_signal(), and do something like


	--- kernel/utrace.c
	+++ kernel/utrace.c
	@@ -390,6 +390,13 @@ static bool utrace_stop(struct task_stru
		 * for stop signals and SIGCONT.
		 */
		spin_lock(&utrace->lock);
	+
	+	return utrace_stop_locked(task, utrace, report);
	+}
	+
	+bool utrace_stop_locked(struct task_struct *task, struct utrace *utrace,
	+			bool report)
	+{
		spin_lock_irq(&task->sighand->siglock);
	 
		if (unlikely(sigismember(&task->pending.signal, SIGKILL))) {
	--- kernel/signal.c
	+++ kernel/signal.c
	@@ -1602,6 +1602,16 @@ static int do_signal_stop(int signr)
		do {
			schedule();
		} while (try_to_freeze());
	+
	+	// needs a tracehook helper
	+	if (utrace->stopped) {
	+		spin_lock(&utrace->lock);
	+		if (utrace->stopped)
	+			utrace_stop_locked();
	+		else
	+			spin_unlock(&utrace->lock);
	+	}
	+
		/*
		 * Now we don't run again until continued.
		 */

No?

Oleg.