linux-kernel - Re: [PATCH 17/17] ptrace: implement PTRACE

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110602173330.GA20384@redhat.com>
Date:	Thu, 2 Jun 2011 19:33:30 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	vda.linux@...glemail.com, jan.kratochvil@...hat.com,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	akpm@...ux-foundation.org, indan@....nu, bdonlan@...il.com,
	pedro@...esourcery.com
Subject: Re: [PATCH 17/17] ptrace: implement PTRACE_LISTEN

On 05/30, Tejun Heo wrote:
>
> This allows ptracer to monitor group stop state without running tracee
> - use INTERRUPT to put tracee into STOP trap, issue LISTEN and then
> wait(2) to wait for the next group stop event.  When it happens,
> PTRACE_GETSIGINFO provides information to determine the current state.

Great. Just a couple of questions,

> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1416,7 +1416,7 @@ static int wait_task_stopped(struct wait_opts *wo,
>  	spin_lock_irq(&p->sighand->siglock);
>
>  	p_code = task_stopped_code(p, ptrace);
> -	if (unlikely(!p_code))
> +	if (unlikely(!p_code) || p->jobctl & JOBCTL_LISTENING)
>  		goto unlock_sig;

Up to you, but perhaps this JOBCTL_LISTENING check should go into
task_stopped_code() ? Or do you think we can't check it without
siglock?

> +	case PTRACE_LISTEN:
> +		/*
> +		 * Listen for events.  Tracee must be in STOP.  It's not
> +		 * resumed per-se but is not considered to be in TRACED by
> +		 * wait(2) or ptrace(2).  If an async event (e.g. group
> +		 * stop state change) happens, tracee will enter STOP trap
> +		 * again.  Alternatively, ptracer can issue INTERRUPT to
> +		 * finish listening and re-trap tracee into STOP.
> +		 */
> +		if (unlikely(!seized || !lock_task_sighand(child, &flags)))
> +			break;
> +
> +		si = child->last_siginfo;
> +		if (unlikely(!si || si->si_code != PTRACE_STOP_SI_CODE))
> +			break;
> +
> +		child->jobctl |= JOBCTL_LISTENING;
> +
> +		/*
> +		 * If NOTIFY is set, it means event happened between start
> +		 * of this trap and now.  Trigger re-trap immediately.
> +		 */
> +		if (child->jobctl & JOBCTL_TRAP_NOTIFY)
> +			signal_wake_up(child, true);

Again, I won't insist if you prefer signal_wake_up(), but afaics
wake_up_state(__TASK_TRACED) should be enough.

> @@ -838,7 +840,7 @@ static void ptrace_trap_notify(struct task_struct *t)
>  	assert_spin_locked(&t->sighand->siglock);
>
>  	task_set_jobctl_pending(t, JOBCTL_TRAP_NOTIFY);
> -	signal_wake_up(t, 0);
> +	signal_wake_up(t, t->jobctl & JOBCTL_LISTENING);
>  }

OK. The only thing I can't understand is why prepare_signal(SIGCONT)
calls ptrace_trap_notify() unconditionally. How about

		if (likely(!(t->ptrace & PT_SEIZED)))
			wake_up_state(t, __TASK_STOPPED);
	-	else
	+	else if (why)
			ptrace_trap_notify(t);

?

Oleg.

--
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/