Skip to contentSkip to conversation

Deny CLONE_PARENT|CLONE_NEWPID|CLONE_SIGHAND combination

RFC PATCH In review Sukadev Bhattiprolu 5 participants 6 messages Jul 1, 2009 – Jul 11, 2009 1 message not in the archive Namespaces:

lore
Share & tools
lore

Sukadev Bhattiprolu

author
#0 lore raw

RFC PATCH 1/1 Deny CLONE_PARENT|CLONE_NEWPID|CLONE_SIGHAND combination

Deny CLONE_PARENT|CLONE_NEWPID|CLONE_SIGHAND combination.

CLONE_PARENT was used to implement an older threading model. For consistency with the CLONE_THREAD check in copy_pid_ns(), disable CLONE_PARENT and CLONE_SIGHAND with CLONE_NEWPID, at least until the required semantics of the pid namespaces are clear.

Changelog[v2]:
[Eric Biederman] Disable CLONE_SIGHAND also ???

Signed-off-by Sukadev Bhattiprolu <sukadev-r/[email protected]>
Acked-by Roland McGrath <[email protected]>
---
 kernel/pid_namespace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/pid_namespace.c +1 -1
Index: linux-mmotm/kernel/pid_namespace.c
===================================================================
@@ -118,7 +118,7 @@ struct pid_namespace *copy_pid_ns(unsign
 {
 	if (!(flags & CLONE_NEWPID))
 		return get_pid_ns(old_ns);
-	if (flags & CLONE_THREAD)
+	if (flags & (CLONE_THREAD|CLONE_PARENT|CLONE_SIGHAND))
 		return ERR_PTR(-EINVAL);
 	return create_pid_namespace(old_ns);
 }

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

Acked-by
#2 lore raw
Quoted 8 lines · Sukadev Bhattiprolu

Deny CLONE_PARENT|CLONE_NEWPID|CLONE_SIGHAND combination.

CLONE_PARENT was used to implement an older threading model. For consistency with the CLONE_THREAD check in copy_pid_ns(), disable CLONE_PARENT and CLONE_SIGHAND with CLONE_NEWPID, at least until the required semantics of the pid namespaces are clear.

Quoted 5 lines

Changelog[v2]:
[Eric Biederman] Disable CLONE_SIGHAND also ???

Signed-off-by Sukadev Bhattiprolu <sukadev-r/[email protected]>
Acked-by Roland McGrath <[email protected]>
Acked-by Serge Hallyn <serue-r/[email protected]>
Quoted 17 lines
---
 kernel/pid_namespace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-mmotm/kernel/pid_namespace.c
===================================================================

kernel/pid_namespace.c +1 -1
@@ -118,7 +118,7 @@ struct pid_namespace *copy_pid_ns(unsign
 {
 	if (!(flags & CLONE_NEWPID))
 		return get_pid_ns(old_ns);
-	if (flags & CLONE_THREAD)
+	if (flags & (CLONE_THREAD|CLONE_PARENT|CLONE_SIGHAND))
 		return ERR_PTR(-EINVAL);
 	return create_pid_namespace(old_ns);
 }
#3 lore raw
In reply to a message not in the archive
Quoted 22 lines · Sukadev Bhattiprolu

Deny CLONE_PARENT|CLONE_NEWPID|CLONE_SIGHAND combination.

CLONE_PARENT was used to implement an older threading model. For consistency with the CLONE_THREAD check in copy_pid_ns(), disable CLONE_PARENT and CLONE_SIGHAND with CLONE_NEWPID, at least until the required semantics of the pid namespaces are clear.

Changelog[v2]:
[Eric Biederman] Disable CLONE_SIGHAND also ???

Signed-off-by Sukadev Bhattiprolu <sukadev-r/[email protected]>
Acked-by Roland McGrath <[email protected]>
---
 kernel/pid_namespace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-mmotm/kernel/pid_namespace.c
===================================================================

kernel/pid_namespace.c struct pid_namespace *copy_pid_ns(unsign · line 122

Quoted 6 lines
kernel/pid_namespace.c +1 -1
@@ -118,7 +118,7 @@ struct pid_namespace *copy_pid_ns(unsign
 {
 	if (!(flags & CLONE_NEWPID))
 		return get_pid_ns(old_ns);
-	if (flags & CLONE_THREAD)
+	if (flags & (CLONE_THREAD|CLONE_PARENT|CLONE_SIGHAND))
 		return ERR_PTR(-EINVAL);

Still can't understand why CLONE_SIGHAND is forbidden...

CLONE_SIGHAND doesn't mean we share the signal queue, so what is the reason?

Oleg.

Acked-by on v1 1/1
#4 lore raw
In reply to Sukadev Bhattiprolu
Quoted 8 lines · Sukadev Bhattiprolu

Deny CLONE_PARENT|CLONE_NEWPID|CLONE_SIGHAND combination.

CLONE_PARENT was used to implement an older threading model. For consistency with the CLONE_THREAD check in copy_pid_ns(), disable CLONE_PARENT and CLONE_SIGHAND with CLONE_NEWPID, at least until the required semantics of the pid namespaces are clear.

Quoted 5 lines

Changelog[v2]:
[Eric Biederman] Disable CLONE_SIGHAND also ???

Signed-off-by Sukadev Bhattiprolu <[email protected]>
Acked-by Roland McGrath <[email protected]>
Acked-by Serge Hallyn <[email protected]>
Quoted 17 lines
---
 kernel/pid_namespace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-mmotm/kernel/pid_namespace.c
===================================================================

kernel/pid_namespace.c +1 -1
@@ -118,7 +118,7 @@ struct pid_namespace *copy_pid_ns(unsign
 {
 	if (!(flags & CLONE_NEWPID))
 		return get_pid_ns(old_ns);
-	if (flags & CLONE_THREAD)
+	if (flags & (CLONE_THREAD|CLONE_PARENT|CLONE_SIGHAND))
 		return ERR_PTR(-EINVAL);
 	return create_pid_namespace(old_ns);
 }
#5 lore raw
In reply to Sukadev Bhattiprolu
Quoted 22 lines · Sukadev Bhattiprolu

Deny CLONE_PARENT|CLONE_NEWPID|CLONE_SIGHAND combination.

CLONE_PARENT was used to implement an older threading model. For consistency with the CLONE_THREAD check in copy_pid_ns(), disable CLONE_PARENT and CLONE_SIGHAND with CLONE_NEWPID, at least until the required semantics of the pid namespaces are clear.

Changelog[v2]:
[Eric Biederman] Disable CLONE_SIGHAND also ???

Signed-off-by Sukadev Bhattiprolu <[email protected]>
Acked-by Roland McGrath <[email protected]>
---
 kernel/pid_namespace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-mmotm/kernel/pid_namespace.c
===================================================================

kernel/pid_namespace.c struct pid_namespace *copy_pid_ns(unsign · line 122

Quoted 6 lines
kernel/pid_namespace.c +1 -1
@@ -118,7 +118,7 @@ struct pid_namespace *copy_pid_ns(unsign
 {
 	if (!(flags & CLONE_NEWPID))
 		return get_pid_ns(old_ns);
-	if (flags & CLONE_THREAD)
+	if (flags & (CLONE_THREAD|CLONE_PARENT|CLONE_SIGHAND))
 		return ERR_PTR(-EINVAL);

Still can't understand why CLONE_SIGHAND is forbidden...

CLONE_SIGHAND doesn't mean we share the signal queue, so what is the reason?

Oleg.

author
#6 lore raw

Oleg Nesterov [[email protected]] wrote:

| On 07/01, Sukadev Bhattiprolu wrote:
| >
| > Deny CLONE_PARENT|CLONE_NEWPID|CLONE_SIGHAND combination.
| >
| > CLONE_PARENT was used to implement an older threading model.  For consistency
| > with the CLONE_THREAD check in copy_pid_ns(), disable CLONE_PARENT and
| > CLONE_SIGHAND with CLONE_NEWPID, at least until the required semantics of
| > the pid namespaces are clear.
| >
| > Changelog[v2]:
| > 	[Eric Biederman] Disable CLONE_SIGHAND also ???
| >
| > Signed-off-by: Sukadev Bhattiprolu <[email protected]>
| > Acked-by: Roland McGrath <[email protected]>
| > ---
| >  kernel/pid_namespace.c |    2 +-
| >  1 file changed, 1 insertion(+), 1 deletion(-)
| >
| > Index: linux-mmotm/kernel/pid_namespace.c
| > ===================================================================
| > --- linux-mmotm.orig/kernel/pid_namespace.c	2009-06-30 23:01:09.000000000 -0700
| > +++ linux-mmotm/kernel/pid_namespace.c	2009-06-30 23:49:06.000000000 -0700
| > @@ -118,7 +118,7 @@ struct pid_namespace *copy_pid_ns(unsign
| >  {
| >  	if (!(flags & CLONE_NEWPID))
| >  		return get_pid_ns(old_ns);
| > -	if (flags & CLONE_THREAD)
| > +	if (flags & (CLONE_THREAD|CLONE_PARENT|CLONE_SIGHAND))
| >  		return ERR_PTR(-EINVAL);
| 
| Still can't understand why CLONE_SIGHAND is forbidden...
| 
| CLONE_SIGHAND doesn't mean we share the signal queue, so what is
| the reason?

I don't have a convincing reason. We could disable just the CLONE_NEWPID|CLONE_PARENT combination till we have a need for it. We talked about removing CLONE_PARENT but backed-off since some application may be using it. Disabling specific combinations like this (CLONE_NEWPID is relatively a new flag) could make it a little easier to remove CLONE_PARENT in the future.

Sukadev