linux-kernel - Re: [PATCH 04/14] signal: don't notify parent if not stopping after tracehook_notify_jctl() in do_signal

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: <20101126144654.GB28177@redhat.com>
Date:	Fri, 26 Nov 2010 15:46:54 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	roland@...hat.com, linux-kernel@...r.kernel.org,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	"rjw@...k.plpavel"@ucw.cz
Subject: Re: [PATCH 04/14] signal: don't notify parent if not stopping
	after tracehook_notify_jctl() in do_signal_stop()

On 11/26, Tejun Heo wrote:
>
> do_signal_stop() tests sig->group_stop_count one more time after
> calling tracehook_notify_jctl() as it's allowed release siglock.  If
> group_stop_count has changed to zero, it no longer stops but still
> notifies the parent.

Only if tracehook_notify_jctl() returns nonzero. That was the
point, tracehook_ can ask to notify even if we are not going to
stop.

> Also, tracehook_notify_jctl() doesn't release siglock, so, currently,
> none of these matters at all.

Yes. But with this patch it is not possible to drop siglock in
tracehook_notify_jctl().

> This doesn't cause any behavior difference as the condition never
> triggers in the current code.

I don't understand the motivation then (probably I should read
the next patches).

If we kill the ability to drop ->siglock in tracehook_notify_jctl(),
we can simplify the code further. No need to start with
->group_stop_count = 1 and then decrement it again.

We could do something like

	static int do_signal_stop(int signr)
	{
		struct signal_struct *sig = current->signal;
		int notify;

		if (sig->group_stop_count) {
			--sig->group_stop_count;
		} else {
			struct task_struct *t;

			if (!likely(sig->flags & SIGNAL_STOP_DEQUEUED) ||
			    unlikely(signal_group_exit(sig)))
				return 0;
			/*
			 * There is no group stop already in progress.
			 * We must initiate one now.
			 */
			sig->group_exit_code = signr;

			for (t = next_thread(current); t != current; t = next_thread(t))
				/*
				 * Setting state to TASK_STOPPED for a group
				 * stop is always done with the siglock held,
				 * so this check has no races.
				 */
				if (!(t->flags & PF_EXITING) &&
				    !task_is_stopped_or_traced(t)) {
					sig->group_stop_count++;
					signal_wake_up(t, 0);
				}
		}

		if (!sig->group_stop_count) {
			sig->flags = SIGNAL_STOP_STOPPED;
			current->exit_code = sig->group_exit_code;
			__set_current_state(TASK_STOPPED);
			notify = CLD_STOPPED;
		}
		spin_unlock_irq(&current->sighand->siglock);

		if (notify || current->ptrace) {
			read_lock(&tasklist_lock);
			do_notify_parent_cldstop(current, notify);
			read_unlock(&tasklist_lock);
		}

		/* Now we don't run again until woken by SIGCONT or SIGKILL */
		schedule();

		tracehook_finish_jctl();
		current->exit_code = 0;

		return 1;
	}

In short, this patch dismisses tracehook_notify_jctl().

Roland?

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/