Skip to content

exec_id/exit_signal fixes

Oleg Nesterov kernel-hardening linux-fsdevel lkml 1 version New

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest3 patches 0 repliesNew

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3exit_signal: simplify the "we have changed execution domain" logicfs/exec.c kernel/exit.c+4 -60 replies
2/3exit_signal: fix the "parent has changed security domain" logickernel/exit.c kernel/signal.c+9 -140 replies
3/3exec: move de_thread()->setmax_mm_hiwater_rss() into exec_mmap()fs/exec.c+1 -30 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/3] exec_id/exit_signal fixes

Andrew, these changes are orthogonal to "CLONE_PARENT shouldn't allow to set ->exit_signal" in -mm.

Please review/comment. The patches are really simple I hope, but with or without them I do not understand eligible_child().

	/* Wait for all children (clone and not) if __WALL is set;
	 * otherwise, wait for clone children *only* if __WCLONE is
	 * set; otherwise, wait for non-clone children *only*.  (Note:
	 * A "clone" child here is one that reports to its parent
	 * using a signal other than SIGCHLD.) */
	if (((p->exit_signal != SIGCHLD) ^ !!(wo->wo_flags & __WCLONE))
	    && !(wo->wo_flags & __WALL))
		return 0;

OK, but exec/exit can change ->exit_signal "in between".

For example. The parent clones the !SIGCHLD child and does do_wait(__WCLONE). The child execs. The parent still sleeps until child exits but do_wait() returns ECHILD. And more, this also depends on who does exec in mt case.

Or the parent execs. In this case eligible_child() depends on when the child exits, before or after exec.

Anyway, the current check in exit_notify() is wrong, 2/2 tries to fix this. But both patches add the (hopefully minor) user-visible changes wrt eligible_child().

Really, I think de_thread() should set ->exit_signal = SIGCHLD for each child. This looks simple and understandable, and with the CLONE_PARENT change above we can kill self_exec_id/parent_exec_id.

May be de_thread() should also do __wake_up_parent() if it changes ->exit_signal. Or perhaps we should change eligible_child() to check p->cloned_with_not_SIGCHLD. I dunno. I hope this doesn't matter and nobody cares.

OTOH, I never understood why we should reset ->exit_signal if the child execs but the parent is still the same.

3/3 is offtopic minor cleanup. Imho setmax_mm_hiwater_rss() in de_thread() looks strange.

Oleg.