Skip to contentSkip to conversation

signals: replace p->pid == 1 check with a check for task_child_reaper

PATCH In review Daniel Hokka Zakrisson 4 participants 7 messages Jul 17, 2008 2 messages not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 signals: replace p->pid == 1 check with a check for task_child_reaper

p->pid == 1 is insufficient when there are multiple pid namespaces. Instead, check whether the task is in the current task's child reaper's thread group.

kernel/signal.c +0 -0
index 93713a5..be932b9 100644
@@ -1142,10 +1142,20 @@ static int kill_something_info(int sig, struct 

siginfo *info, int pid)
pid ? find_vpid(-pid) : task_pgrp(current));
} else {
int retval = 0, count = 0;
- struct task_struct * p;
+ struct task_struct *p, *reaper = task_child_reaper(current);
+
+ /*
+ * The reaper has died, so there's probably a
+ * SIGKILL pending. Return.
+ */
+ if (unlikely(!reaper)) {
+ ret = -ESRCH;
+ goto out;
+ }

for_each_process(p) {
- if (p->pid > 1 && !same_thread_group(p, current) &&
+ if (!same_thread_group(p, reaper) &&
+ !same_thread_group(p, current) &&
task_in_pid_ns(p, current->nsproxy->pid_ns)) {
int err = group_send_sig_info(sig, info, p); ++count;
@@ -1155,6 +1165,7 @@ static int kill_something_info(int sig, struct siginfo *info, int pid)
}
ret = count ? retval : -ESRCH;
}
+out:
read_unlock(&tasklist_lock);

return ret;

Signature · 1 line
1.5.5.1

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 3 lines · Daniel Hokka Zakrisson

p->pid == 1 is insufficient when there are multiple pid namespaces. Instead, check whether the task is in the current task's child reaper's thread group.

We should just drop the check for init as it is redundant.

Eric

#3 lore raw
In reply to Daniel Hokka Zakrisson
Quoted 3 lines · Daniel Hokka Zakrisson

p->pid == 1 is insufficient when there are multiple pid namespaces. Instead, check whether the task is in the current task's child reaper's thread group.

We should just drop the check for init as it is redundant.

Eric

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw
Quoted 7 lines · Eric W. Biederman
Quoted 3 lines · Daniel Hokka Zakrisson

p->pid == 1 is insufficient when there are multiple pid namespaces. Instead, check whether the task is in the current task's child reaper's thread group.

We should just drop the check for init as it is redundant.

I'm not sure what you mean? Without protecting init here, kill -s 9 -- -1 will kill it (i.e. the init in the pid namespace). E.g.:

# vspace --new --pid --mount -- bash
# bash -c 'kill -s 9 -- -1'
will kill off all those processes, and dispose of the pid namespace.
Quoted 1 line

Eric

Signature · 1 line
Daniel Hokka Zakrisson
#6 lore raw
In reply to Eric W. Biederman
Quoted 7 lines · Eric W. Biederman
Quoted 3 lines · Daniel Hokka Zakrisson

p->pid == 1 is insufficient when there are multiple pid namespaces. Instead, check whether the task is in the current task's child reaper's thread group.

We should just drop the check for init as it is redundant.

I'm not sure what you mean? Without protecting init here, kill -s 9 -- -1 will kill it (i.e. the init in the pid namespace). E.g.:

# vspace --new --pid --mount -- bash
# bash -c 'kill -s 9 -- -1'
will kill off all those processes, and dispose of the pid namespace.
Quoted 1 line

Eric

Signature · 1 line
Daniel Hokka Zakrisson
#7 lore raw
Quoted 8 lines · Daniel Hokka Zakrisson
Quoted 7 lines · Eric W. Biederman
Quoted 3 lines · Daniel Hokka Zakrisson

p->pid == 1 is insufficient when there are multiple pid namespaces. Instead, check whether the task is in the current task's child reaper's thread group.

We should just drop the check for init as it is redundant.

Sorry that was a half truth. Outside of the context of pid namespaces it is true.

In the context of pid namespaces it is false because we haven't merged the patches to drop signals from inside the pid namespace on the way to init.

So it is a check that _should_ be redundant.

Eric

#8 lore raw
In reply to Daniel Hokka Zakrisson
Quoted 8 lines · Daniel Hokka Zakrisson
Quoted 7 lines · Eric W. Biederman
Quoted 3 lines · Daniel Hokka Zakrisson

p->pid == 1 is insufficient when there are multiple pid namespaces. Instead, check whether the task is in the current task's child reaper's thread group.

We should just drop the check for init as it is redundant.

Sorry that was a half truth. Outside of the context of pid namespaces it is true.

In the context of pid namespaces it is false because we haven't merged the patches to drop signals from inside the pid namespace on the way to init.

So it is a check that _should_ be redundant.

Eric