Re: [PATCH 1/2] signals: kill(-1) should only signal processes inthe same namespace

From: sukadev
Date: Thu Jul 17 2008 - 14:14:35 EST


Daniel Hokka Zakrisson [daniel@xxxxxxxxx] wrote:
| While moving Linux-VServer to using pid namespaces, I noticed that
| kill(-1) from inside a pid namespace is currently signalling every
| process in the entire system, including processes that are otherwise
| unreachable from the current process.
|
| This patch fixes it by making sure that only processes which are in
| the same pid namespace as current get signalled.
|
| Signed-off-by: Daniel Hokka Zakrisson <daniel@xxxxxxxxx>
|
| diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
| index caff528..4cf41bd 100644
| --- a/include/linux/pid_namespace.h
| +++ b/include/linux/pid_namespace.h
| @@ -40,6 +40,8 @@ static inline struct pid_namespace *get_pid_ns(struct
| pid_namespace *ns)
| extern struct pid_namespace *copy_pid_ns(unsigned long flags, struct
| pid_namespace *ns);
| extern void free_pid_ns(struct kref *kref);
| extern void zap_pid_ns_processes(struct pid_namespace *pid_ns);
| +extern int task_in_pid_ns(struct task_struct *tsk,
| + struct pid_namespace *pid_ns);
|
| static inline void put_pid_ns(struct pid_namespace *ns)
| {
| @@ -72,6 +74,12 @@ static inline void zap_pid_ns_processes(struct
| pid_namespace *ns)
| {
| BUG();
| }
| +
| +static inline int task_in_pid_ns(struct task_struct *tsk,
| + struct pid_namespace *ns)
| +{
| + return 1;
| +}
| #endif /* CONFIG_PID_NS */
|
| static inline struct pid_namespace *task_active_pid_ns(struct
| task_struct *tsk)
| diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
| index 98702b4..3e71011 100644
| --- a/kernel/pid_namespace.c
| +++ b/kernel/pid_namespace.c
| @@ -188,6 +188,26 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
| return;
| }
|
| +/*
| + * Checks whether tsk has a pid in the pid namespace ns.
| + * Must be called with tasklist_lock read-locked or under rcu_read_lock()
| + */
| +int task_in_pid_ns(struct task_struct *tsk, struct pid_namespace *ns)
| +{
| + struct pid *pid = task_pid(tsk);
| +
| + if (!pid)
| + return 0;
| +
| + if (pid->level < ns->level)
| + return 0;

ns can be NULL if tsk is exiting.

Like Pavel said, we had couple of attempts to fix the larger problem of
signal semantics in containers but did not have a consensus on handling
blocked/unhandled signals to container-init.

It would still be good to fix this "kill -1" problem.

Eric had a slightly optimized interface, 'pid_in_pid_ns()' in following
patchset. Maybe we could use that ?

https://lists.linux-foundation.org/pipermail/containers/2007-December/009174.html


| +
| + if (pid->numbers[ns->level].ns != ns)
| + return 0;
| +
| + return 1;
| +}
| +
| static __init int pid_namespaces_init(void)
| {
| pid_ns_cachep = KMEM_CACHE(pid_namespace, SLAB_PANIC);
| diff --git a/kernel/signal.c b/kernel/signal.c
| index 6c0958e..93713a5 100644
| --- a/kernel/signal.c
| +++ b/kernel/signal.c
| @@ -1145,7 +1145,8 @@ static int kill_something_info(int sig, struct
| siginfo *info, int pid)
| struct task_struct * p;
|
| for_each_process(p) {
| - if (p->pid > 1 && !same_thread_group(p, current)) {
| + if (p->pid > 1 && !same_thread_group(p, current) &&
| + task_in_pid_ns(p, current->nsproxy->pid_ns)) {
| int err = group_send_sig_info(sig, info, p);
| ++count;
| if (err != -EPERM)
| --
| 1.5.5.1
| _______________________________________________
| Containers mailing list
| Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
| https://lists.linux-foundation.org/mailman/listinfo/containers
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/