Thread (1 message) 1 message, 1 author, 2017-06-07

Re: [PATCH 00/26] Fixing wait, exit, ptrace, exec, and CLONE_THREAD

From: Eric W. Biederman <hidden>
Date: 2017-06-07 16:06:15
Also in: lkml

Linus Torvalds [off-list ref] writes:
On Tue, Jun 6, 2017 at 12:01 PM, Eric W. Biederman
[off-list ref] wrote:
quoted
I am posting this patches in the hope of some review of the strategy I
am taking and to let the individual patches be reviewed.
I'm trying to look through these, and finding (as usual) that the
signal handling and exit code is extremely scary from a correctness
and security standpoint.

I really want Oleg to review/ack these. Oleg?
Definitely.  The more review I can get the better. 
I also would really really want to see the stuff that actually changes
semantics split out.

For example, I feel much less nervous about things like making the
tasklist RCU-safe. So I'd like to see changes like that be separated
out from the much scarier ones. Would that be possible? Hint hint..
The patches that I posted are the ones that I would really like to have
ready for the 4.13 merge window.  They are supposed to be the least
scary patches that don't really depend on anything else, and the semantic changes.

After a first brush with code review the non-scary patches wind up just
being these.

[PATCH 01/26] alpha: Remove unused TASK_GROUP_LEADER
[PATCH 02/26] cgroup: Don't open code tasklist_empty()
[PATCH 05/26] exit:  Remove the pointless clearing of SIGPENDING in __exit_signal
[PATCH 07/26] pidns: Improve the error handling in alloc_pid
[PATCH 08/26] exit: Make the runqueue rcu safe

This turns out to have a dependency I overlooked so I am dropping it for now.
[PATCH 06/26] rlimit: Remove unnecessary grab of tasklist_lock

There is a small pair of a semantic change and it's dependency:
[PATCH 03/26] signal: Do not perform permission checks when sending pdeath_signal
[PATCH 04/26] signal: Make group_send_sig_info static

There is a teeny tiny semantic change:
[PATCH 09/26] signal: Don't allow sending SIGKILL or SIGSTOP to init

The deeply related scarier changes (+ indicates it includes a semantic change):
 [PATCH 10/26] ptrace: Simplify ptrace_detach & exit_ptrace
+[PATCH 11/26] wait: Properly implement __WCLONE handling in the presence of exec and ptrace
 [PATCH 12/26] wait: Directly test for the two cases where wait_task_zombie is called
 [PATCH 13/26] wait: Remove unused delay_group_leader
+[PATCH 14/26] wait: Move changing of ptrace from wait_consider_task into wait_task_stopped
+[PATCH 15/26] wait: Don't delay !ptrace_reparented leaders
+[PATCH 16/26] exit: Fix reporting a ptraced !reparented leader has exited
 [PATCH 17/26] exit: Rework the exit states for ptracees
+[PATCH 18/26] wait: Fix WSTOPPED on a ptraced child
 [PATCH 19/26] wait: Simpler code for clearing notask_error in wait_consider_task
 [PATCH 20/26] wait: Don't pass the list to wait_consider_task
 [PATCH 21/26] wait: Optmize waitpid
+[PATCH 22/26] exit: Fix auto-wait of ptraced children
+[PATCH 23/26] signal: Fix SIGCONT before group stop completes.
+[PATCH 24/26] signal: In ptrace_stop improve identical signal detection
+[PATCH 25/26] signal: In ptrace_stop use CLD_TRAPPED in all ptrace signals
+[PATCH 26/26] pidns: Ensure zap_pid_ns_processes always terminates

Out of my 170 or so total changes those are the bulk of the semantic
changes and definitely the scariest.  Even those above are very
conservative and are really just about sorting out the weirdness in the
semantics when we are ptracing our own child which causes wait and the
siginfo of SIGCHLD to have two sets of meanings.

I am hoping we can just get 1,2,5,7, and 8 reviewed and I can just apply
them to my for-next branch.

If I need to repost I will respect the split-out I have described above.

At this point I am happy to see that people are not scared when I
suggest killing the concept of the thread group leader.

Eric
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help