IPC initialize shmmax and shmall from the current value not the default

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

Quoted 2 lines · Marian Marinov

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

Why is this a good idea?

This would break userspace that relies on the current behavior. Furthermore we've recently changed the default value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this being replaced by namespaces.

Thanks,
Davidlohr

Quoted 6 lines · Davidlohr Bueso

Quoted 2 lines · Marian Marinov

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

Why is this a good idea?

Quoted 8 lines

This would break userspace that relies on the current behavior. Furthermore we've recently changed the default value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this being replaced by namespaces.

Thanks,
Davidlohr

The current behavior is create_ipc_ns()->shm_init_ns()

void shm_init_ns(struct ipc_namespace *ns)

{
     ns->shm_ctlmax = SHMMAX;
     ns->shm_ctlall = SHMALL;
     ns->shm_ctlmni = SHMMNI;
     ns->shm_rmid_forced = 0;
     ns->shm_tot = 0;
     ipc_init_ids(&shm_ids(ns));
}

This means that whenever you are creating an IPC namespace it gets its SHMMAX and SHMALL values from the defaults for the kernel.

If for some reason you want to have smaller(or bigger, for older kernels) limit. This means changing the values in /proc/sys/kernel/shmmax and /proc/sys/kernel/shmall. However the program that is started with the new IPC namespace may lack privileges to write to these files and so it can not modify them.

What I'm proposing is simply to copy the current values of the host machine, as set by a privileged process before the namespace creation.

Maybe a better approach would be to allow the changes to be done by processes having CAP_SYS_RESOURCE inside the new namespace?

Quoted 28 lines · Marian Marinov

Quoted 6 lines · Davidlohr Bueso

Quoted 2 lines · Marian Marinov

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

Why is this a good idea?

Quoted 8 lines

This would break userspace that relies on the current behavior. Furthermore we've recently changed the default value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this being replaced by namespaces.

Thanks,
Davidlohr

The current behavior is create_ipc_ns()->shm_init_ns()

void shm_init_ns(struct ipc_namespace *ns)

{
      ns->shm_ctlmax = SHMMAX;
      ns->shm_ctlall = SHMALL;
      ns->shm_ctlmni = SHMMNI;
      ns->shm_rmid_forced = 0;
      ns->shm_tot = 0;
      ipc_init_ids(&shm_ids(ns));
}

Quoted 2 lines

This means that whenever you are creating an IPC namespace it gets its SHMMAX and SHMALL values from the defaults for the kernel.

This is exactly what I meant by 'current behavior'.

Quoted 3 lines

If for some reason you want to have smaller(or bigger, for older kernels) limit. This means changing the values in /proc/sys/kernel/shmmax and /proc/sys/kernel/shmall. However the program that is started with the new IPC namespace may lack privileges to write to these files and so it can not modify them.

I see no reason why namespaces should behave any different than the rest of the system, wrt this. And this changes how *and* when these limits are set, which impacts at a userspace level with no justification.

Quoted 5 lines

What I'm proposing is simply to copy the current values of the host machine, as set by a privileged process before the namespace creation.

Maybe a better approach would be to allow the changes to be done by processes having CAP_SYS_RESOURCE inside the new namespace?

Why do you need this? Is there any real impact/issue you're seeing?

Quoted 2 lines · Marian Marinov

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

Why is this a good idea?

This would break userspace that relies on the current behavior. Furthermore we've recently changed the default value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this being replaced by namespaces.

Hi Marian,

Note: The limits will soon be increased to (nearly) ULONG_MAX. I.e.: If you propose the patch because you are running into issues with a too small SEMMAX after an unshare(CLONE_NEWIPC), then this will be fixed soon.

Quoted 3 lines · Davidlohr Bueso

Quoted 2 lines · Marian Marinov

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

The idea sounds reasonable:
If an admin has reduced the limits, then the reduction should also apply after a unshare(CLONE_NEWIPC).

But:
Your patch doesn't use the current shmmax, it uses the shmmax from init_ipc_ns.
Would it be possible to use the current values?

Quoted 6 lines

Why is this a good idea?

This would break userspace that relies on the current behavior. Furthermore we've recently changed the default value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this being replaced by namespaces.

Davidlohr: We are not deprecating them, we make the default huge. The limits should stay as usable as they were.

With regards to breaking user space, I must think about it a bit more. Right now, each new namespace starts with SEMMAX=32MB, i.e. an often unusable default.

--
Manfred

Hi Marian,

Note: The limits will soon be increased to (nearly) ULONG_MAX.
I.e.: If you propose the patch because you are running into issues with a too small SEMMAX after an unshare(CLONE_NEWIPC), then this will be fixed soon.

Quoted 1 line · Davidlohr Bueso

On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:

Quoted 2 lines

Quoted 2 lines

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

The idea sounds reasonable:
If an admin has reduced the limits, then the reduction should also apply after a unshare(CLONE_NEWIPC).

But:
Your patch doesn't use the current shmmax, it uses the shmmax from init_ipc_ns. Would it be possible to use the current values?

Quoted 7 lines

Why is this a good idea?

This would break userspace that relies on the current behavior. Furthermore we've recently changed the default value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this being replaced by namespaces.

Davidlohr: We are not deprecating them, we make the default huge. The limits should stay as usable as they were.

With regards to breaking user space, I must think about it a bit more. Right now, each new namespace starts with SEMMAX=32MB, i.e. an often unusable default.

--
Manfred

Quoted 7 lines · Manfred Spraul

Hi Marian,

Note: The limits will soon be increased to (nearly) ULONG_MAX. I.e.: If you propose the patch because you are running into issues with a too small SEMMAX after an unshare(CLONE_NEWIPC), then this will be fixed soon.

On 05/04/2014 01:53 AM, Davidlohr Bueso wrote:

Quoted 8 lines

Quoted 3 lines · Marian Marinov

Quoted 2 lines

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

The idea sounds reasonable: If an admin has reduced the limits, then the reduction should also apply after a unshare(CLONE_NEWIPC).

But: Your patch doesn't use the current shmmax, it uses the shmmax from init_ipc_ns. Would it be possible to use the current values?

In my tests it worked exactly as expected. Here is an example:

[root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax = 34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#

So it seams to work as expected :)

It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are creating a new ipc_ns its ok to copy the values from init_ipc_ns.

-Marian

Quoted 6 lines

Quoted 6 lines

Why is this a good idea?

This would break userspace that relies on the current behavior. Furthermore we've recently changed the default value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this being replaced by namespaces.

Quoted 7 lines

Davidlohr: We are not deprecating them, we make the default huge. The limits should stay as usable as they were.

With regards to breaking user space, I must think about it a bit more. Right now, each new namespace starts with SEMMAX=32MB, i.e. an often unusable default.

-- Manfred

Quoted 7 lines · Manfred Spraul

Hi Marian,

Note: The limits will soon be increased to (nearly) ULONG_MAX. I.e.: If you propose the patch because you are running into issues with a too small SEMMAX after an unshare(CLONE_NEWIPC), then this will be fixed soon.

On 05/04/2014 01:53 AM, Davidlohr Bueso wrote:

Quoted 8 lines

Quoted 3 lines · Marian Marinov

Quoted 2 lines

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the values of the current shmmax and shmall to the new namespace.

The idea sounds reasonable: If an admin has reduced the limits, then the reduction should also apply after a unshare(CLONE_NEWIPC).

But: Your patch doesn't use the current shmmax, it uses the shmmax from init_ipc_ns. Would it be possible to use the current values?

In my tests it worked exactly as expected. Here is an example:

[root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax = 34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#

So it seams to work as expected :)

It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are creating a new ipc_ns its ok to copy the values from init_ipc_ns.

-Marian

Quoted 6 lines

Quoted 6 lines

Why is this a good idea?

This would break userspace that relies on the current behavior. Furthermore we've recently changed the default value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this being replaced by namespaces.

Quoted 7 lines

Davidlohr: We are not deprecating them, we make the default huge. The limits should stay as usable as they were.

With regards to breaking user space, I must think about it a bit more. Right now, each new namespace starts with SEMMAX=32MB, i.e. an often unusable default.

-- Manfred

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 11 lines · Marian Marinov

In my tests it worked exactly as expected. Here is an example:

[root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax = 34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#

So it seams to work as expected :)

Quoted 5 lines

It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are creating a new ipc_ns its ok to copy the values from init_ipc_ns.

-Marian

Ping?

So will there be any more comments on that?

Hi Marian,

Quoted 24 lines · Marian Marinov

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

Quoted 9 lines · Marian Marinov

In my tests it worked exactly as expected. Here is an example:

[root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax = 34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#

Quoted 7 lines

So it seams to work as expected :)

It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are creating a new ipc_ns its ok to copy the values from init_ipc_ns.

-Marian

Ping?

So will there be any more comments on that?

Attached is an untested idea: - each new namespace copies from it's parent, i.e. nested namespaces should work. - msg, sem and shm updated

-- Manfred

Hi Marian,

Quoted 22 lines · Marian Marinov

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 9 lines · Marian Marinov

In my tests it worked exactly as expected. Here is an example:

[root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax = 34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#

Quoted 7 lines

So it seams to work as expected :)

It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are creating a new ipc_ns its ok to copy the values from init_ipc_ns.

-Marian

Ping?

Quoted 3 lines

So will there be any more comments on that?

Attached is an untested idea:
- each new namespace copies from it's parent, i.e. nested namespaces should work.
- msg, sem and shm updated

--
Manfred

From ed73ce838fc3f55e34041591a72b3135ccaa460b Mon Sep 17 00:00:00 2001
From: Manfred Spraul <[email protected]>
Date: Sun, 25 May 2014 21:04:42 +0200
Subject: [PATCH] ipc namespace: copy settings from parent namespace

Right now, each IPC namespace starts with the kernel boot standard settings.
This patch changes that:
Now each new namespace starts with the settings from the parent namespace.

The patch updates msg, sem and shm.

Marian: Would that patch help you?

It's just a proposal - not yet tested.

Open issues:
- auto_msgmni is not yet taken from parent.

--
Manfred

---
 ipc/msg.c       | 13 +++++++++----
 ipc/namespace.c |  6 +++---
 ipc/sem.c       | 19 +++++++++++++------
 ipc/shm.c       | 19 +++++++++++++------
 ipc/util.h      | 12 ++++++------
 5 files changed, 44 insertions(+), 25 deletions(-)

ipc/msg.c +9 -4

index 6498531..6c72d43 100644
@@ -107,10 +107,15 @@ void recompute_msgmni(struct ipc_namespace *ns)
 	ns->msg_ctlmni = allowed;
 }
 
-void msg_init_ns(struct ipc_namespace *ns)
+void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
 {
-	ns->msg_ctlmax = MSGMAX;
-	ns->msg_ctlmnb = MSGMNB;
+	if (old_ns != NULL) {
+		ns->msg_ctlmax = old_ns->msg_ctlmax;
+		ns->msg_ctlmnb = old_ns->msg_ctlmnb;
+	} else {
+		ns->msg_ctlmax = MSGMAX;
+		ns->msg_ctlmnb = MSGMNB;
+	}
 
 	recompute_msgmni(ns);
 

@@ -129,7 +134,7 @@ void msg_exit_ns(struct ipc_namespace *ns)
 
 void __init msg_init(void)
 {
-	msg_init_ns(&init_ipc_ns);
+	msg_init_ns(&init_ipc_ns, NULL);
 
 	printk(KERN_INFO "msgmni has been set to %d\n",
 		init_ipc_ns.msg_ctlmni);

ipc/namespace.c +3 -3

index 59451c1..57b65fa 100644
@@ -41,9 +41,9 @@ static struct ipc_namespace *create_ipc_ns(struct user_namespace *user_ns,
 	}
 	atomic_inc(&nr_ipc_ns);
 
-	sem_init_ns(ns);
-	msg_init_ns(ns);
-	shm_init_ns(ns);
+	sem_init_ns(ns, old_ns);
+	msg_init_ns(ns, old_ns);
+	shm_init_ns(ns, old_ns);
 
 	/*
 	 * msgmni has already been computed for the new ipc ns.

ipc/sem.c +13 -6

index bee5554..0e232e6 100644
@@ -170,12 +170,19 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
 #define sc_semopm	sem_ctls[2]
 #define sc_semmni	sem_ctls[3]
 
-void sem_init_ns(struct ipc_namespace *ns)
+void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
 {
-	ns->sc_semmsl = SEMMSL;
-	ns->sc_semmns = SEMMNS;
-	ns->sc_semopm = SEMOPM;
-	ns->sc_semmni = SEMMNI;
+	if (old_ns != NULL) {
+		ns->sc_semmsl = old_ns->sc_semmsl;
+		ns->sc_semmns = old_ns->sc_semmns;
+		ns->sc_semopm = old_ns->sc_semopm;
+		ns->sc_semmni = old_ns->sc_semmni;
+	} else {
+		ns->sc_semmsl = SEMMSL;
+		ns->sc_semmns = SEMMNS;
+		ns->sc_semopm = SEMOPM;
+		ns->sc_semmni = SEMMNI;
+	}
 	ns->used_sems = 0;
 	ipc_init_ids(&ns->ids[IPC_SEM_IDS]);
 }

@@ -190,7 +197,7 @@ void sem_exit_ns(struct ipc_namespace *ns)
 
 void __init sem_init(void)
 {
-	sem_init_ns(&init_ipc_ns);
+	sem_init_ns(&init_ipc_ns, NULL);
 	ipc_init_proc_interface("sysvipc/sem",
 				"       key      semid perms      nsems   uid   gid  cuid  cgid      otime      ctime\n",
 				IPC_SEM_IDS, sysvipc_sem_proc_show);

ipc/shm.c +13 -6

index 7645961..5c54b25 100644
@@ -72,12 +72,19 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp);
 static int sysvipc_shm_proc_show(struct seq_file *s, void *it);
 #endif
 
-void shm_init_ns(struct ipc_namespace *ns)
+void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
 {
-	ns->shm_ctlmax = SHMMAX;
-	ns->shm_ctlall = SHMALL;
-	ns->shm_ctlmni = SHMMNI;
-	ns->shm_rmid_forced = 0;
+	if (old_ns != NULL) {
+		ns->shm_ctlmax = old_ns->shm_ctlmax;
+		ns->shm_ctlall = old_ns->shm_ctlall;
+		ns->shm_ctlmni = old_ns->shm_ctlmni;
+		ns->shm_rmid_forced = old_ns->shm_rmid_forced;
+	} else {
+		ns->shm_ctlmax = SHMMAX;
+		ns->shm_ctlall = SHMALL;
+		ns->shm_ctlmni = SHMMNI;
+		ns->shm_rmid_forced = 0;
+	}
 	ns->shm_tot = 0;
 	ipc_init_ids(&shm_ids(ns));
 }

@@ -110,7 +117,7 @@ void shm_exit_ns(struct ipc_namespace *ns)
 
 static int __init ipc_ns_init(void)
 {
-	shm_init_ns(&init_ipc_ns);
+	shm_init_ns(&init_ipc_ns, NULL);
 	return 0;
 }
 

ipc/util.h +6 -6

index 9c47d6f..bb62b44 100644
@@ -30,17 +30,17 @@ static inline void mq_put_mnt(struct ipc_namespace *ns) { }
 #endif
 
 #ifdef CONFIG_SYSVIPC
-void sem_init_ns(struct ipc_namespace *ns);
-void msg_init_ns(struct ipc_namespace *ns);
-void shm_init_ns(struct ipc_namespace *ns);
+void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
+void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
+void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
 
 void sem_exit_ns(struct ipc_namespace *ns);
 void msg_exit_ns(struct ipc_namespace *ns);
 void shm_exit_ns(struct ipc_namespace *ns);
 #else
-static inline void sem_init_ns(struct ipc_namespace *ns) { }
-static inline void msg_init_ns(struct ipc_namespace *ns) { }
-static inline void shm_init_ns(struct ipc_namespace *ns) { }
+static inline void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
+static inline void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
+static inline void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
 
 static inline void sem_exit_ns(struct ipc_namespace *ns) { }
 static inline void msg_exit_ns(struct ipc_namespace *ns) { }

Signature · 8 lines

1.9.0



_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers