linux-kernel - Re: Kernel threads

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: <20070309205205.GA173@tv-sign.ru>
Date:	Fri, 9 Mar 2007 23:52:05 +0300
From:	Oleg Nesterov <oleg@...sign.ru>
To:	Roland McGrath <roland@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-os (Dick Johnson)" <linux-os@...logic.com>,
	linux-kernel@...r.kernel.org
Subject: Re: Kernel threads

On 03/08, Roland McGrath wrote:
>
> Your change seems fine to me.  I certainly concur that it seems insane
> for init to be responsible for tasks created magically inside the
> kernel.  The history I've found says that the setting to SIGCHLD was
> introduced as part of "v2.5.1.9 -> v2.5.1.10", without detailed
> commentary in the log.  This was probably before the auto-reaping
> semantics worked as they do now.  So like the man said, at the time,
> it seemed the logical thing to do.
>
> To be paranoid, I wouldn't make this change in any stable kernel series.
> It changes behavior visible to userland (init) from how it has been
> consistently for five years, so, who knows, something might notice.
> The old behavior is pretty harmless, albeit changing it seems both
> preferable and harmless.

Yes sure, this change shoud be tested in -mm tree (I'll send the patch
on Sunday after some testing). The only (afaics) problem is that with
this change a kernel thread must not do do_fork(CLONE_THREAD). I think
it should not, but currently this is technically possible. Perhaps it
makes sense to add BUG_ON(CLONE_THREAD && group_leader->exit_signal==-1)
in copy_process().

On a related note,

	zap_other_threads:

		if (t != p->group_leader)
			t->exit_signal = -1;

looks like another leftover to me, we already depend on the fact that
all sub-threads have ->exit_signal == -1 (otherwise, for example, a
thread group just can't exit properly).

While we are talking about kernel threads, there is something I can't
undestand. kthread/daemonize use sigprocmask(SIG_BLOCK) to protect
against signals. This doesn't look right to me, because this doesn't
prevent the signal delivery, this only blocks signal_wake_up(). Every
"killall -33 khelper" means a "struct siginfo" leak.

Imho, the kernel thread shouldn't play with ->blocked at all. Instead
it should set SIG_IGN for all handlers. If it really needs, say, SIGCHLD,
it should call allow_signal() anyway. Do you see any problems with this
approach?

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/