OpenVZ Forum


Home » Mailing lists » Devel » [PATCH] Masquerade sender information
[PATCH] Masquerade sender information [message #22419] Sat, 27 October 2007 19:02 Go to next message
Sukadev Bhattiprolu is currently offline  Sukadev Bhattiprolu
Messages: 413
Registered: August 2006
Senior Member
From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
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@us.ibm.com>
---
 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
===================================================================
--- 2.6.23-mm1.orig/include/linux/pid.h	2007-10-27 08:48:56.000000000 -0700
+++ 2.6.23-mm1/include/linux/pid.h	2007-10-27 09:56:53.000000000 -0700
@@ -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
===================================================================
--- 2.6.23-mm1.orig/kernel/pid.c	2007-10-27 08:50:51.000000000 -0700
+++ 2.6.23-mm1/kernel/pid.c	2007-10-27 10:03:28.000000000 -0700
@@ -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
===================================================================
--- 2.6.23-mm1.orig/kernel/signal.c	2007-10-27 08:50:51.000000000 -0700
+++ 2.6.23-mm1/kernel/signal.c	2007-10-27 10:02:04.000000000 -0700
@@ -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
===================================================================
--- 2.6.23-mm1.orig/include/linux/pid_namespace.h	2007-10-27 09:44:25.000000000 -0700
+++ 2.6.23-mm1/include/linux/pid_namespace.h	2007-10-27 10:04:20.000000000 -0700
@@ -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 */
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] Masquerade sender information [message #22486 is a reply to message #22419] Mon, 29 October 2007 20:06 Go to previous messageGo to next message
ebiederm is currently offline  ebiederm
Messages: 1354
Registered: February 2006
Senior Member
sukadev@us.ibm.com writes:

> From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> 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
we also want si_pid = 0.

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

Eric


_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] Masquerade sender information [message #22706 is a reply to message #22486] Thu, 01 November 2007 16:50 Go to previous messageGo to next message
serue is currently offline  serue
Messages: 750
Registered: February 2006
Senior Member
Quoting Eric W. Biederman (ebiederm@xmission.com):
> sukadev@us.ibm.com writes:
> 
> > From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> > 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

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

> we also want si_pid = 0.
> 
> If that holds this problem is easier then I was thinking it would
> be.
> 
> Eric
> 
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] Masquerade sender information [message #22707 is a reply to message #22419] Thu, 01 November 2007 16:59 Go to previous messageGo to next message
ebiederm is currently offline  ebiederm
Messages: 1354
Registered: February 2006
Senior Member
sukadev@us.ibm.com writes:

> +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;
}

Eric
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] Masquerade sender information [message #22709 is a reply to message #22419] Thu, 01 November 2007 17:03 Go to previous messageGo to next message
Pavel Emelianov is currently offline  Pavel Emelianov
Messages: 1149
Registered: September 2006
Senior Member
sukadev@us.ibm.com wrote:
> From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> 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@us.ibm.com>

Hmm... Definitely looks good. I thought that the problem was that 
the siginfo structure is not always allocated, but hey!, if we
failed to allocate it, then it doesn't really matter whether we 
masquerade something or not! Good point!

> ---
>  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
> ===================================================================
> --- 2.6.23-mm1.orig/include/linux/pid.h	2007-10-27 08:48:56.000000000 -0700
> +++ 2.6.23-mm1/include/linux/pid.h	2007-10-27 09:56:53.000000000 -0700
> @@ -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
> ===================================================================
> --- 2.6.23-mm1.orig/kernel/pid.c	2007-10-27 08:50:51.000000000 -0700
> +++ 2.6.23-mm1/kernel/pid.c	2007-10-27 10:03:28.000000000 -0700
> @@ -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
> ===================================================================
> --- 2.6.23-mm1.orig/kernel/signal.c	2007-10-27 08:50:51.000000000 -0700
> +++ 2.6.23-mm1/kernel/signal.c	2007-10-27 10:02:04.000000000 -0700
> @@ -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
> ===================================================================
> --- 2.6.23-mm1.orig/include/linux/pid_namespace.h	2007-10-27 09:44:25.000000000 -0700
> +++ 2.6.23-mm1/include/linux/pid_namespace.h	2007-10-27 10:04:20.000000000 -0700
> @@ -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 */
> 

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] Masquerade sender information [message #22737 is a reply to message #22706] Fri, 02 November 2007 13:40 Go to previous messageGo to next message
Cedric Le Goater is currently offline  Cedric Le Goater
Messages: 443
Registered: February 2006
Senior Member
Serge E. Hallyn wrote:
> Quoting Eric W. Biederman (ebiederm@xmission.com):
>> sukadev@us.ibm.com writes:
>>
>>> From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
>>> 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
> 
> 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.

C. 

> 
>> we also want si_pid = 0.
>>
>> If that holds this problem is easier then I was thinking it would
>> be.
>>
>> Eric
>>

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] Masquerade sender information [message #22739 is a reply to message #22737] Fri, 02 November 2007 13:45 Go to previous messageGo to next message
serue is currently offline  serue
Messages: 750
Registered: February 2006
Senior Member
Quoting Cedric Le Goater (clg@fr.ibm.com):
> Serge E. Hallyn wrote:
> > Quoting Eric W. Biederman (ebiederm@xmission.com):
> >> sukadev@us.ibm.com writes:
> >>
> >>> From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> >>> 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
> > 
> > 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.

And I think I'm fine with that, I was just wondering about Eric's claim
that only the kernel can send signals from inside a pidns to something
not in a child pidns.  We can treat these cases as being from the
kernel, but it's not in fact the case that the signals came from the
kernel.

-serge
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] Masquerade sender information [message #22741 is a reply to message #22707] Fri, 02 November 2007 14:05 Go to previous messageGo to next message
Cedric Le Goater is currently offline  Cedric Le Goater
Messages: 443
Registered: February 2006
Senior Member
Eric W. Biederman wrote:
> sukadev@us.ibm.com writes:
> 
>> +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;
> }

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.

C.
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] Masquerade sender information [message #22805 is a reply to message #22741] Sun, 04 November 2007 04:12 Go to previous message
ebiederm is currently offline  ebiederm
Messages: 1354
Registered: February 2006
Senior Member
Cedric Le Goater <clg@fr.ibm.com> writes:

> Eric W. Biederman wrote:
>> sukadev@us.ibm.com writes:
>> 
>>> +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;
>> }
>
> 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.

Yes.  However you are currently missing the case where the target pid
namespace is a parent pid namespace.  So besides applying the change
to liberally we also missed the case when sending to a parent pid
namespace.  task_tgid_nr_ns(current, t->nsporxy->pid_ns) handles that.

Technically I think that is safe right now, but I think I would like
to pass in the task_struct of the sender because we have a few odd
instances where current is not the sender although every case I have
traced we do continue to be in the same process group.

Eric
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Previous Topic: problem compiling 2.6.18 with 028stab47
Next Topic: [PATCH] namespaces: introduce sys_hijack (v8)
Goto Forum:
  


Current Time: Tue Jun 25 05:31:44 GMT 2024

Total time taken to generate the page: 0.02751 seconds