Masquerade sender information

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>
Subject: [PATCH] Masquerade sender information

With multiple pid namespaces, sender of a signal could be in an ancestor namespace of the receiver and so the sender will not have a valid 'pid_t' in the receiver's namespace.

In this case, masquerade the 'siginfo' for the signal to pretend that the signal originated from the kernel.

Quoted 7 lines · sukadev-r/[email protected] writes:

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>
Subject: [PATCH] Masquerade sender information

With multiple pid namespaces, sender of a signal could be in an ancestor namespace of the receiver and so the sender will not have a valid 'pid_t' in the receiver's namespace.

Quoted 2 lines

In this case, masquerade the 'siginfo' for the signal to pretend that the signal originated from the kernel.

At first glance this looks ok. I think the only case where we can be sending a signal from inside a pid namespace to something not in a child pid namespace is if we are the kernel. In which case

we also want si_pid = 0.

If that holds this problem is easier then I was thinking it would be.

Eric

Quoted 9 lines · Eric W. Biederman

Quoted 7 lines · sukadev-r/[email protected] writes:

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>
Subject: [PATCH] Masquerade sender information

With multiple pid namespaces, sender of a signal could be in an ancestor namespace of the receiver and so the sender will not have a valid 'pid_t' in the receiver's namespace.

Quoted 5 lines

Quoted 2 lines

In this case, masquerade the 'siginfo' for the signal to pretend that the signal originated from the kernel.

At first glance this looks ok. I think the only case where we can be sending a signal from inside a pid namespace to something not in a child pid namespace is if we are the kernel. In which case

Are we now blocking F_SETOWN|F_SETSIG signals to outside our pid namespace? mq_notify? (I didn't think we were)

Quoted 8 lines

we also want si_pid = 0.

If that holds this problem is easier then I was thinking it would be.

Eric

Quoted 16 lines · Serge E. Hallyn

Quoted 14 lines · Eric W. Biederman

Quoted 9 lines · sukadev-r/[email protected] writes:

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>
Subject: [PATCH] Masquerade sender information

With multiple pid namespaces, sender of a signal could be in an ancestor namespace of the receiver and so the sender will not have a valid 'pid_t' in the receiver's namespace.

In this case, masquerade the 'siginfo' for the signal to pretend that the signal originated from the kernel.

At first glance this looks ok. I think the only case where we can be sending a signal from inside a pid namespace to something not in a child pid namespace is if we are the kernel. In which case

Quoted 2 lines

Are we now blocking F_SETOWN|F_SETSIG signals to outside our pid namespace? mq_notify? (I didn't think we were)

My understanding is that we're not blokcing and that a process killing another process in a sibling pid namespace will have a si_pid = 0.

kernel/signal.c static void handle_stop_signal(int sig, · line 694

Quoted 13 lines · sukadev-r/[email protected] writes:

kernel/signal.c +13 -0

+static void masquerade_sender(struct task_struct *t, struct sigqueue *q)
+{
+       /*
+        * If the sender does not have a pid_t in the receiver's active
+        * pid namespace, set si_pid to 0 and pretend signal originated
+        * from the kernel.
+        */
+       if (!pid_ns_equal(t)) {
+               q->info.si_pid = 0;
+               q->info.si_uid = 0;
+               q->info.si_code = SI_KERNEL;
+       }
+}

It looks like we are hooked in the right place. However the way we are handling this appears wrong.

First. If we have an si_code that does not use si_pid then we should not be changing si_pid, because the structure is a union and that field is not always a pid value.

My gut feel says the code should be something like:

switch (q->info->si_code & __SI_MASK) {
case __SI_KILL:
case __SI_CHILD:
case __SI_RT:
case __MESQ:

       q->info->si_pid = task_pid_nr_ns(current, t->nsproxy->pid_ns);
       break;
}

Quoted 26 lines · Eric W. Biederman

kernel/signal.c static void handle_stop_signal(int sig, · line 694

Quoted 13 lines · sukadev-r/[email protected] writes:

kernel/signal.c +13 -0

+static void masquerade_sender(struct task_struct *t, struct sigqueue *q)
+{
+       /*
+        * If the sender does not have a pid_t in the receiver's active
+        * pid namespace, set si_pid to 0 and pretend signal originated
+        * from the kernel.
+        */
+       if (!pid_ns_equal(t)) {
+               q->info.si_pid = 0;
+               q->info.si_uid = 0;
+               q->info.si_code = SI_KERNEL;
+       }
+}

It looks like we are hooked in the right place. However the way we are handling this appears wrong.

First. If we have an si_code that does not use si_pid then we should not be changing si_pid, because the structure is a union and that field is not always a pid value.

My gut feel says the code should be something like:

Quoted 8 lines

switch (q->info->si_code & __SI_MASK) {
case __SI_KILL:
case __SI_CHILD:
case __SI_RT:
case __MESQ:

       q->info->si_pid = task_pid_nr_ns(current, t->nsproxy->pid_ns);
       break;
}

IMHO, it should be

	q->info->si_pid = 0.

we're trying to cover the case where the sender does not have a pid_t in the receiver's active pid namespace.

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>
Subject: [PATCH] Masquerade sender information

With multiple pid namespaces, sender of a signal could be in an ancestor namespace of the receiver and so the sender will not have a valid 'pid_t' in the receiver's namespace.

In this case, masquerade the 'siginfo' for the signal to pretend that the signal originated from the kernel.

Signed-off-by Sukadev Bhattiprolu <sukadev-r/[email protected]>

---
 include/linux/pid.h           |    8 ++++++++
 include/linux/pid_namespace.h |    1 +
 kernel/pid.c                  |   13 +++++++++++++
 kernel/signal.c               |   15 +++++++++++++++
 4 files changed, 37 insertions(+)

Index: 2.6.23-mm1/include/linux/pid.h
===================================================================

include/linux/pid.h +8 -0

@@ -123,6 +123,14 @@ extern struct pid *alloc_pid(struct pid_
 extern void FASTCALL(free_pid(struct pid *pid));
 extern void zap_pid_ns_processes(struct pid_namespace *pid_ns);
 
+static inline struct pid_namespace *pid_active_ns(struct pid *pid)
+{
+       if (!pid)
+               return NULL;
+
+       return pid->numbers[pid->level].ns;
+}
+
 /*
  * the helpers to get the pid's id seen from different namespaces
  *

Index: 2.6.23-mm1/kernel/pid.c
===================================================================

kernel/pid.c +13 -0

@@ -430,6 +430,19 @@ struct pid *find_get_pid(pid_t nr)
 	return pid;
 }
 
+/*
+ * Return TRUE if the active pid namespace of @tsk is same as active
+ * pid namespace of 'current'.
+ *
+ * Note the difference between this and the task_in_pid_ns() below.
+ * task_in_pid_ns() includes processes in descendant pid name spaces
+ * but pid_ns_equal() only matches _active_ pid namespaces.
+ */
+int pid_ns_equal(struct task_struct *tsk)
+{
+	return pid_active_ns(task_pid(current)) == pid_active_ns(task_pid(tsk));
+}
+
 static int pid_in_pid_ns(struct pid *pid, struct pid_namespace *ns)
 {
 	return pid && (ns->level <= pid->level) &&

Index: 2.6.23-mm1/kernel/signal.c
===================================================================

kernel/signal.c +15 -0

@@ -679,6 +679,20 @@ static void handle_stop_signal(int sig, 
 	}
 }
 
+static void masquerade_sender(struct task_struct *t, struct sigqueue *q)
+{
+       /*
+        * If the sender does not have a pid_t in the receiver's active
+        * pid namespace, set si_pid to 0 and pretend signal originated
+        * from the kernel.
+        */
+       if (!pid_ns_equal(t)) {
+               q->info.si_pid = 0;
+               q->info.si_uid = 0;
+               q->info.si_code = SI_KERNEL;
+       }
+}
+
 static int send_signal(int sig, struct siginfo *info, struct task_struct *t,
 			struct sigpending *signals)
 {

@@ -730,6 +744,7 @@ static int send_signal(int sig, struct s
 			copy_siginfo(&q->info, info);
 			break;
 		}
+		masquerade_sender(t, q);
 	} else if (!is_si_special(info)) {
 		if (sig >= SIGRTMIN && info->si_code != SI_USER)
 		/*

Index: 2.6.23-mm1/include/linux/pid_namespace.h
===================================================================

include/linux/pid_namespace.h +1 -0

@@ -56,6 +56,7 @@ static inline struct task_struct *task_c
 	return tsk->nsproxy->pid_ns->child_reaper;
 }
 
+extern int pid_ns_equal(struct task_struct *tsk);
 extern int task_in_pid_ns(struct task_struct *tsk, struct pid_namespace *ns);
 
 #endif /* _LINUX_PID_NS_H */