OpenVZ Forum - RDF feed
type: channel
items: [PATCH] IPC namespace
items: [PATCH 1/6] IPC namespace core
items: [PATCH 2/6] IPC namespace - utils
items: [PATCH 3/6] IPC namespace - msg
items: [PATCH 4/6] IPC namespace - sem
items: [PATCH 5/6] IPC namespace - shm
items: [PATCH 6/6] IPC namespace - sysctls
items: Re: [PATCH 1/6] IPC namespace core
items: Re: [PATCH 2/6] IPC namespace - utils
items: Re: [PATCH] IPC namespace

[PATCH] IPC namespace
type: item
description: The patches in this thread add IPC namespace functionality additionally to already included in -mm tree UTS namespace. This patch set allows to unshare IPCs and have a private set of IPC objects (sem, shm, msg) inside namespace. Basically, it is another building block of containers functionality. Tested with LTP inside namespaces. Signed-Off-By: Pavel Emelianiov <xemul@openvz.org> Signed-Off-By: Kirill Korotaev <dev@openvz.org> P.S. patches are against linux-2.6.17-rc6-mm1
creator: Kirill Korotaev
date: 2006-06-09T14:55:48-00:00

[PATCH 1/6] IPC namespace core
type: item
description: This patch implements core IPC namespace changes: - ipc_namespace structure - new config option CONFIG_IPC_NS - adds CLONE_NEWIPC flag - unshare support Signed-Off-By: Pavel Emelianov <xemul@openvz.org> Signed-Off-By: Kirill Korotaev <dev@openvz.org> --- ./include/linux/init_task.h.ipcns 2006-06-06 14:47:58.000000000 +0400 +++ ./include/linux/init_task.h 2006-06-08 14:28:23.000000000 +0400 @@ -73,6 +73,7 @@ extern struct nsproxy init_nsproxy; .count = ATOMIC_INIT(1), \ .nslock = SPIN_LOCK_UNLOCKED, \ .uts_ns = &init_uts_ns, \ + .ipc_ns = &init_ipc_ns, \ .namespace = NULL, \ } --- ./include/linux/ipc.h.ipcns 2006-04-21 11:59:36.000000000 +0400 +++ ./include/linux/ipc.h 2006-06-08 15:43:43.000000000 +0400 @@ -2,6 +2,7 @@ #define _LINUX_IPC_H #include <linux/types.h> +#include <linux/kref.h> #define IPC_PRIVATE ((__kernel_key_t) 0) @@ -68,6 +69,41 @@ struct kern_ipc_perm void *security; }; +struct ipc_ids; +struct ipc_namespace { + struct kref kref; + struct ipc_ids *ids[3]; + + int sem_ctls[4]; + int used_sems; + + int msg_ctlmax; + int msg_ctlmnb; + int msg_ctlmni; + + size_t shm_ctlmax; + size_t shm_ctlall; + int shm_ctlmni; + int shm_tot; +}; + +extern struct ipc_namespace init_ipc_ns; +extern void free_ipc_ns(struct kref *kref); +extern int copy_ipcs(unsigned long flags, struct task_struct *tsk); +extern int unshare_ipcs(unsigned long flags, struct ipc_namespace **ns); + +static inline struct ipc_namespace *get_ipc_ns(struct ipc_namespace *ns) +{ + if (ns) + kref_get(&ns->kref); + return ns; +} + +static inline void put_ipc_ns(struct ipc_namespace *ns) +{ + kref_put(&ns->kref, free_ipc_ns); +} + #endif /* __KERNEL__ */ #endif /* _LINUX_IPC_H */ --- ./include/linux/nsproxy.h.ipcns 2006-06-06 14:47:58.000000000 +0400 +++ ./include/linux/nsproxy.h 2006-06-08 15:28:02.000000000 +0400 @@ -6,6 +6,7 @@ struct namespace; struct uts_namespace; +struct ipc_namespace; /* * A structure to contain pointers to all per-process @@ -23,6 +24,7 @@ struct nsproxy { atomic_t count; s…
creator: Kirill Korotaev
date: 2006-06-09T15:01:38-00:00

[PATCH 2/6] IPC namespace - utils
type: item
description: This patch adds basic IPC namespace functionality to IPC utils: - init_ipc_ns - copy/clone/unshare/free IPC ns - /proc preparations Signed-Off-By: Pavel Emelianov <xemul@openvz.org> Signed-Off-By: Kirill Korotaev <dev@openvz.org> --- ./ipc/util.c.ipcns 2006-05-18 12:05:15.000000000 +0400 +++ ./ipc/util.c 2006-06-09 14:19:30.000000000 +0400 @@ -12,6 +12,9 @@ * Mingming Cao <cmm@us.ibm.com> * Mar 2006 - support for audit of ipc object properties * Dustin Kirkland <dustin.kirkland@us.ibm.com> + * Jun 2006 - namespaces ssupport + * OpenVZ, SWsoft Inc. + * Pavel Emelianov <xemul@openvz.org> */ #include <linux/config.h> @@ -30,6 +33,7 @@ #include <linux/seq_file.h> #include <linux/proc_fs.h> #include <linux/audit.h> +#include <linux/nsproxy.h> #include <asm/unistd.h> @@ -38,10 +42,126 @@ struct ipc_proc_iface { const char *path; const char *header; - struct ipc_ids *ids; + int ids; int (*show)(struct seq_file *, void *); }; +struct ipc_namespace init_ipc_ns = { + .kref = { + .refcount = ATOMIC_INIT(2), + }, +}; + +#ifdef CONFIG_IPC_NS +static struct ipc_namespace *clone_ipc_ns(struct ipc_namespace *old_ns) +{ + int err; + struct ipc_namespace *ns; + + err = -ENOMEM; + ns = kmalloc(sizeof(struct ipc_namespace), GFP_KERNEL); + if (ns == NULL) + goto err_mem; + + err = sem_init_ns(ns); + if (err) + goto err_sem; + err = msg_init_ns(ns); + if (err) + goto err_msg; + err = shm_init_ns(ns); + if (err) + goto err_shm; + + kref_init(&ns->kref); + return ns; + +err_shm: + msg_exit_ns(ns); +err_msg: + sem_exit_ns(ns); +err_sem: + kfree(ns); +err_mem: + return ERR_PTR(err); +} + +int unshare_ipcs(unsigned long unshare_flags, struct ipc_namespace **new_ipc) +{ + struct ipc_namespace *new; + + if (unshare_flags & CLONE_NEWIPC) { + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + new = clone_ipc_ns(current->nsproxy->ipc_ns); + if (IS_ERR(new)) + return PTR_ERR(new); + + *new_ipc = new; + } + + return 0; +} + +int copy_ipcs(unsigned long flags, struct task_struct *tsk) +{ + struct ipc_…
creator: Kirill Korotaev
date: 2006-06-09T15:05:29-00:00

[PATCH 3/6] IPC namespace - msg
type: item
description: IPC namespace support for IPC msg code. Signed-Off-By: Pavel Emelianiov <xemul@openvz.org> Signed-Off-By: Kirill Korotaev <dev@openvz.org> --- ./ipc/msg.c.ipcns 2006-06-06 14:47:58.000000000 +0400 +++ ./ipc/msg.c 2006-06-09 14:20:57.000000000 +0400 @@ -16,6 +16,10 @@ * * support for audit of ipc object properties and permission changes * Dustin Kirkland <dustin.kirkland@us.ibm.com> + * + * namespaces support + * OpenVZ, SWsoft Inc. + * Pavel Emelianov <xemul@openvz.org> */ #include <linux/capability.h> @@ -32,16 +36,12 @@ #include <linux/audit.h> #include <linux/seq_file.h> #include <linux/mutex.h> +#include <linux/nsproxy.h> #include <asm/current.h> #include <asm/uaccess.h> #include "util.h" -/* sysctl: */ -int msg_ctlmax = MSGMAX; -int msg_ctlmnb = MSGMNB; -int msg_ctlmni = MSGMNI; - /* one msg_receiver structure for each sleeping receiver */ struct msg_receiver { struct list_head r_list; @@ -68,32 +68,75 @@ struct msg_sender { static atomic_t msg_bytes = ATOMIC_INIT(0); static atomic_t msg_hdrs = ATOMIC_INIT(0); -static struct ipc_ids msg_ids; +static struct ipc_ids init_msg_ids; -#define msg_lock(id) ((struct msg_queue*)ipc_lock(&msg_ids,id)) -#define msg_unlock(msq) ipc_unlock(&(msq)->q_perm) -#define msg_rmid(id) ((struct msg_queue*)ipc_rmid(&msg_ids,id)) -#define msg_checkid(msq, msgid) \ - ipc_checkid(&msg_ids,&msq->q_perm,msgid) -#define msg_buildid(id, seq) \ - ipc_buildid(&msg_ids, id, seq) +#define msg_ids(ns) (*((ns)->ids[IPC_MSG_IDS])) -static void freeque (struct msg_queue *msq, int id); -static int newque (key_t key, int msgflg); +#define msg_lock(ns, id) ((struct msg_queue*)ipc_lock(&msg_ids(ns), id)) +#define msg_unlock(msq) ipc_unlock(&(msq)->q_perm) +#define msg_rmid(ns, id) ((struct msg_queue*)ipc_rmid(&msg_ids(ns),id)) +#define msg_checkid(ns, msq, msgid) \ + ipc_checkid(&msg_ids(ns),&msq->q_perm,msgid) +#define msg_buildid(ns, id, seq) \ + ipc_buildid(&msg_ids(ns), id, seq) + +static void freeque (struct ipc_namespace *ns, struct msg_queue *ms…
creator: Kirill Korotaev
date: 2006-06-09T15:07:05-00:00

[PATCH 4/6] IPC namespace - sem
type: item
description: IPC namespace support for IPC sem code. Signed-Off-By: Pavel Emelianiov <xemul@openvz.org> Signed-Off-By: Kirill Korotaev <dev@openvz.org> --- ./ipc/sem.c.ipcns 2006-06-06 14:47:58.000000000 +0400 +++ ./ipc/sem.c 2006-06-09 14:20:06.000000000 +0400 @@ -64,6 +64,10 @@ * * support for audit of ipc object properties and permission changes * Dustin Kirkland <dustin.kirkland@us.ibm.com> + * + * namespaces support + * OpenVZ, SWsoft Inc. + * Pavel Emelianov <xemul@openvz.org> */ #include <linux/config.h> @@ -79,22 +83,25 @@ #include <linux/capability.h> #include <linux/seq_file.h> #include <linux/mutex.h> +#include <linux/nsproxy.h> #include <asm/uaccess.h> #include "util.h" +#define sem_ids(ns) (*((ns)->ids[IPC_SEM_IDS])) -#define sem_lock(id) ((struct sem_array*)ipc_lock(&sem_ids,id)) -#define sem_unlock(sma) ipc_unlock(&(sma)->sem_perm) -#define sem_rmid(id) ((struct sem_array*)ipc_rmid(&sem_ids,id)) -#define sem_checkid(sma, semid) \ - ipc_checkid(&sem_ids,&sma->sem_perm,semid) -#define sem_buildid(id, seq) \ - ipc_buildid(&sem_ids, id, seq) -static struct ipc_ids sem_ids; +#define sem_lock(ns, id) ((struct sem_array*)ipc_lock(&sem_ids(ns), id)) +#define sem_unlock(sma) ipc_unlock(&(sma)->sem_perm) +#define sem_rmid(ns, id) ((struct sem_array*)ipc_rmid(&sem_ids(ns), id)) +#define sem_checkid(ns, sma, semid) \ + ipc_checkid(&sem_ids(ns),&sma->sem_perm,semid) +#define sem_buildid(ns, id, seq) \ + ipc_buildid(&sem_ids(ns), id, seq) -static int newary (key_t, int, int); -static void freeary (struct sem_array *sma, int id); +static struct ipc_ids init_sem_ids; + +static int newary (struct ipc_namespace *, key_t, int, int); +static void freeary (struct ipc_namespace *ns, struct sem_array *sma, int id); #ifdef CONFIG_PROC_FS static int sysvipc_sem_proc_show(struct seq_file *s, void *it); #endif @@ -111,22 +118,61 @@ static int sysvipc_sem_proc_show(struct * */ -int sem_ctls[4] = {SEMMSL, SEMMNS, SEMOPM, SEMMNI}; -#define sc_semmsl (sem_ctls[0]) -#define sc_semmns (sem_ctls[1…
creator: Kirill Korotaev
date: 2006-06-09T15:08:33-00:00

[PATCH 5/6] IPC namespace - shm
type: item
description: IPC namespace support for IPC shm code. Signed-Off-By: Pavel Emelianiov <xemul@openvz.org> Signed-Off-By: Kirill Korotaev <dev@openvz.org> --- ./ipc/shm.c.ipcns 2006-06-06 14:47:58.000000000 +0400 +++ ./ipc/shm.c 2006-06-09 14:25:34.000000000 +0400 @@ -15,6 +15,10 @@ * * support for audit of ipc object properties and permission changes * Dustin Kirkland <dustin.kirkland@us.ibm.com> + * + * namespaces support + * OpenVZ, SWsoft Inc. + * Pavel Emelianov <xemul@openvz.org> */ #include <linux/config.h> @@ -33,6 +37,7 @@ #include <linux/ptrace.h> #include <linux/seq_file.h> #include <linux/mutex.h> +#include <linux/nsproxy.h> #include <asm/uaccess.h> @@ -41,59 +46,115 @@ static struct file_operations shm_file_operations; static struct vm_operations_struct shm_vm_ops; -static struct ipc_ids shm_ids; +static struct ipc_ids init_shm_ids; -#define shm_lock(id) ((struct shmid_kernel*)ipc_lock(&shm_ids,id)) -#define shm_unlock(shp) ipc_unlock(&(shp)->shm_perm) -#define shm_get(id) ((struct shmid_kernel*)ipc_get(&shm_ids,id)) -#define shm_buildid(id, seq) \ - ipc_buildid(&shm_ids, id, seq) +#define shm_ids(ns) (*((ns)->ids[IPC_SHM_IDS])) -static int newseg (key_t key, int shmflg, size_t size); +#define shm_lock(ns, id) \ + ((struct shmid_kernel*)ipc_lock(&shm_ids(ns),id)) +#define shm_unlock(shp) \ + ipc_unlock(&(shp)->shm_perm) +#define shm_get(ns, id) \ + ((struct shmid_kernel*)ipc_get(&shm_ids(ns),id)) +#define shm_buildid(ns, id, seq) \ + ipc_buildid(&shm_ids(ns), id, seq) + +static int newseg (struct ipc_namespace *ns, key_t key, + int shmflg, size_t size); static void shm_open (struct vm_area_struct *shmd); static void shm_close (struct vm_area_struct *shmd); +static void shm_destroy (struct ipc_namespace *ns, struct shmid_kernel *shp); #ifdef CONFIG_PROC_FS static int sysvipc_shm_proc_show(struct seq_file *s, void *it); #endif -size_t shm_ctlmax = SHMMAX; -size_t shm_ctlall = SHMALL; -int shm_ctlmni = SHMMNI; +static void __ipc_init __shm_init_ns(struct ipc_namespace *ns…
creator: Kirill Korotaev
date: 2006-06-09T15:09:56-00:00

[PATCH 6/6] IPC namespace - sysctls
type: item
description: Sysctl tweaks for IPC namespace Signed-Off-By: Pavel Emelianiov <xemul@openvz.org> Signed-Off-By: Kirill Korotaev <dev@openvz.org> --- ./kernel/sysctl.c.ipcns 2006-06-06 14:47:59.000000000 +0400 +++ ./kernel/sysctl.c 2006-06-08 15:29:58.000000000 +0400 @@ -104,13 +104,8 @@ extern char modprobe_path[]; extern int sg_big_buff; #endif #ifdef CONFIG_SYSVIPC -extern size_t shm_ctlmax; -extern size_t shm_ctlall; -extern int shm_ctlmni; -extern int msg_ctlmax; -extern int msg_ctlmnb; -extern int msg_ctlmni; -extern int sem_ctls[]; +static int proc_do_ipc_string(ctl_table *table, int write, struct file *filp, + void __user *buffer, size_t *lenp, loff_t *ppos); #endif #ifdef __sparc__ @@ -510,58 +505,58 @@ static ctl_table kern_table[] = { { .ctl_name = KERN_SHMMAX, .procname = "shmmax", - .data = &shm_ctlmax, + .data = NULL, .maxlen = sizeof (size_t), .mode = 0644, - .proc_handler = &proc_doulongvec_minmax, + .proc_handler = &proc_do_ipc_string, }, { .ctl_name = KERN_SHMALL, .procname = "shmall", - .data = &shm_ctlall, + .data = NULL, .maxlen = sizeof (size_t), .mode = 0644, - .proc_handler = &proc_doulongvec_minmax, + .proc_handler = &proc_do_ipc_string, }, { .ctl_name = KERN_SHMMNI, .procname = "shmmni", - .data = &shm_ctlmni, + .data = NULL, .maxlen = sizeof (int), .mode = 0644, - .proc_handler = &proc_dointvec, + .proc_handler = &proc_do_ipc_string, }, { .ctl_name = KERN_MSGMAX, .procname = "msgmax", - .data = &msg_ctlmax, + .data = NULL, .maxlen = sizeof (int), .mode = 0644, - .proc_handler = &proc_dointvec, + .proc_handler = &proc_do_ipc_string, }, { .ctl_name = KERN_MSGMNI, .procname = "msgmni", - .data = &msg_ctlmni, + .data = NULL, .maxlen = sizeof (int), .mode = 0644, - .proc_handler = &proc_dointvec, + .proc_handler = &proc_do_ipc_string, }, { .ctl_name = KERN_MSGMNB, .procname = "msgmnb", - .data = &msg_ctlmnb, + .data = NULL, .maxlen = sizeof (int), .mode = 0644, - .proc_handler = &proc_dointvec, + .proc_handler = &proc_do_ipc_string, }, { .ctl_name = KERN_SEM,…
creator: Kirill Korotaev
date: 2006-06-09T15:11:07-00:00

Re: [PATCH 1/6] IPC namespace core
type: item
description: Kirill Korotaev wrote: > This patch implements core IPC namespace changes: > - ipc_namespace structure > - new config option CONFIG_IPC_NS > - adds CLONE_NEWIPC flag > - unshare support > > Signed-Off-By: Pavel Emelianov <xemul@openvz.org> > Signed-Off-By: Kirill Korotaev <dev@openvz.org> > > > ------------------------------------------------------------ ------------ > > --- ./include/linux/init_task.h.ipcns 2006-06-06 14:47:58.000000000 +0400 > +++ ./include/linux/init_task.h 2006-06-08 14:28:23.000000000 +0400 > @@ -73,6 +73,7 @@ extern struct nsproxy init_nsproxy; > .count = ATOMIC_INIT(1), \ > .nslock = SPIN_LOCK_UNLOCKED, \ > .uts_ns = &init_uts_ns, \ > + .ipc_ns = &init_ipc_ns, \ > .namespace = NULL, \ > } > > --- ./include/linux/ipc.h.ipcns 2006-04-21 11:59:36.000000000 +0400 > +++ ./include/linux/ipc.h 2006-06-08 15:43:43.000000000 +0400 > @@ -2,6 +2,7 @@ > #define _LINUX_IPC_H > > #include <linux/types.h> > +#include <linux/kref.h> > > #define IPC_PRIVATE ((__kernel_key_t) 0) > > @@ -68,6 +69,41 @@ struct kern_ipc_perm > void *security; > }; > > +struct ipc_ids; > +struct ipc_namespace { > + struct kref kref; > + struct ipc_ids *ids[3]; > + > + int sem_ctls[4]; > + int used_sems; > + > + int msg_ctlmax; > + int msg_ctlmnb; > + int msg_ctlmni; > + > + size_t shm_ctlmax; > + size_t shm_ctlall; > + int shm_ctlmni; > + int shm_tot; > +}; you could probably simplify your patch by moving struct ipc_ids to ipc.h and not allocating ids. see patch bellow. I've been working all week on this patchset :) C.
creator: Cedric Le Goater
date: 2006-06-09T15:20:15-00:00

Re: [PATCH 1/6] IPC namespace core
type: item
description: On Fri, 9 Jun 2006, Kirill Korotaev wrote: > This patch implements core IPC namespace changes: > - ipc_namespace structure > - new config option CONFIG_IPC_NS > - adds CLONE_NEWIPC flag > - unshare support > Please post patches as inline text, so they can be reviewed inline. +struct ipc_namespace { + struct kref kref; + struct ipc_ids *ids[3]; + + int sem_ctls[4]; It'd be nice if these weren't magic numbers. -- James Morris <jmorris@namei.org>
creator: James Morris
date: 2006-06-09T15:26:46-00:00

Re: [PATCH 1/6] IPC namespace core
type: item
description: On Fri, 09 Jun 2006 19:01:38 +0400 Kirill Korotaev <dev@openvz.org> wrote: > --- ./include/linux/init_task.h.ipcns 2006-06-06 14:47:58.000000000 +0400 > +++ ./include/linux/init_task.h 2006-06-08 14:28:23.000000000 +0400 > @@ -73,6 +73,7 @@ extern struct nsproxy init_nsproxy; > .count = ATOMIC_INIT(1), \ > .nslock = SPIN_LOCK_UNLOCKED, \ > .uts_ns = &init_uts_ns, \ > + .ipc_ns = &init_ipc_ns, \ > .namespace = NULL, \ > } Not all the new additions here are inside CONFIG_IPC_NS. It'd be nice to make it so.
creator: Andrew Morton
date: 2006-06-09T18:38:02-00:00

Re: [PATCH 1/6] IPC namespace core
type: item
description: Kirill Korotaev <dev@openvz.org> writes: > --- ./kernel/fork.c.ipcns 2006-06-06 14:47:58.000000000 +0400 > +++ ./kernel/fork.c 2006-06-08 15:31:03.000000000 +0400 > @@ -1592,6 +1592,7 @@ asmlinkage long sys_unshare(unsigned lon > struct sem_undo_list *new_ulist = NULL; > struct nsproxy *new_nsproxy = NULL, *old_nsproxy = NULL; > struct uts_namespace *uts, *new_uts = NULL; > + struct ipc_namespace *ipc, *new_ipc = NULL; > > check_unshare_flags(&unshare_flags); > > @@ -1617,18 +1618,20 @@ asmlinkage long sys_unshare(unsigned lon > goto bad_unshare_cleanup_fd; > if ((err = unshare_utsname(unshare_flags, &new_uts))) > goto bad_unshare_cleanup_semundo; > + if ((err = unshare_ipcs(unshare_flags, &new_ipc))) > + goto bad_unshare_cleanup_uts; > > if (new_ns || new_uts) { This test needs to be updated to test for new_ipc. > old_nsproxy = current->nsproxy; > new_nsproxy = dup_namespaces(old_nsproxy); > if (!new_nsproxy) { > err = -ENOMEM; > - goto bad_unshare_cleanup_uts; > + goto bad_unshare_cleanup_ipc; > } > } Eric
creator: ebiederm
date: 2006-06-10T00:44:30-00:00

Re: [PATCH 1/6] IPC namespace core
type: item
description: On Fri, 09 Jun 2006 19:01:38 +0400 Kirill Korotaev <dev@openvz.org> wrote: > This patch implements core IPC namespace changes: > - ipc_namespace structure > - new config option CONFIG_IPC_NS > - adds CLONE_NEWIPC flag > - unshare support `make allnoconfig' arch/i386/kernel/init_task.o:(.data+0x1a0): undefined reference to `init_ipc_ns' kernel/built-in.o: In function `free_nsproxy': : undefined reference to `free_ipc_ns' kernel/built-in.o: In function `copy_namespaces': : undefined reference to `copy_ipcs'
creator: Andrew Morton
date: 2006-06-10T03:22:58-00:00

Re: [PATCH 2/6] IPC namespace - utils
type: item
description: Kirill Korotaev wrote: > +static struct ipc_namespace *clone_ipc_ns(struct ipc_namespace *old_ns) > +{ > + int err; > + struct ipc_namespace *ns; > + > + err = -ENOMEM; > + ns = kmalloc(sizeof(struct ipc_namespace), GFP_KERNEL); > + if (ns == NULL) > + goto err_mem; > + > + err = sem_init_ns(ns); > + if (err) > + goto err_sem; > + err = msg_init_ns(ns); > + if (err) > + goto err_msg; > + err = shm_init_ns(ns); > + if (err) > + goto err_shm; > + > + kref_init(&ns->kref); > + return ns; > + > +err_shm: > + msg_exit_ns(ns); > +err_msg: > + sem_exit_ns(ns); > +err_sem: > + kfree(ns); > +err_mem: > + return ERR_PTR(err); > +} I've used the ipc namespace patchset in rc6-mm2. Thanks for putting this together, it works pretty well ! A few questions when we clone : * We should do something close to what exit_sem() already does to clear the sem_undo list from the task doing the clone() or unshare(). * I don't like the idea of being able to unshare the ipc namespace and keep some shared memory from the previous ipc namespace mapped in the process mm. Should we forbid the unshare ? Small fix follows, thanks, C.
creator: Cedric Le Goater
date: 2006-06-12T17:08:38-00:00

Re: [PATCH] IPC namespace
type: item
description: On Fri, 2006-06-09 at 18:55 +0400, Kirill Korotaev wrote: > The patches in this thread add IPC namespace functionality > additionally to already included in -mm tree UTS namespace. > > This patch set allows to unshare IPCs and have a private set > of IPC objects (sem, shm, msg) inside namespace. Basically, it is > another building block of containers functionality. > > Tested with LTP inside namespaces. Do you, by chance, have any test cases for this code that test the unsharing itself, and not just the functionality before and after an unshare? -- Dave
creator: Dave Hansen
date: 2006-06-12T17:19:28-00:00

Re: [PATCH 2/6] IPC namespace - utils
type: item
description: Cedric Le Goater <clg@fr.ibm.com> writes: > I've used the ipc namespace patchset in rc6-mm2. Thanks for putting this > together, it works pretty well ! A few questions when we clone : > > * We should do something close to what exit_sem() already does to clear the > sem_undo list from the task doing the clone() or unshare(). Possibly which case are you trying to prevent? > * I don't like the idea of being able to unshare the ipc namespace and keep > some shared memory from the previous ipc namespace mapped in the process mm. > Should we forbid the unshare ? No. As long as the code handles that case properly we should be fine. As a general principle we should be able to keep things from other namespaces open if we get them. The chroot or equivalent binary is the one that needs to ensure these kinds of issues don't exist if we care. Speaking of we should put together a small test application probably similar to chroot so people can access these features at least for testing. > Small fix follows, > > thanks, > > C. Ack. For the unshare fix below. Could you resend this one separately with patch in the subject so Andrew sees it and picks up? > From: Cedric Le Goater <clg@fr.ibm.com> > Subject: ipc namespace : unshare fix > > Signed-off-by: Cedric Le Goater <clg@fr.ibm.com> > > --- > kernel/fork.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > Index: 2.6.17-rc6-mm2/kernel/fork.c > ============================================================ ======= > --- 2.6.17-rc6-mm2.orig/kernel/fork.c > +++ 2.6.17-rc6-mm2/kernel/fork.c > @@ -1599,7 +1599,8 @@ asmlinkage long sys_unshare(unsigned lon > /* Return -EINVAL for all unsupported flags */ > err = -EINVAL; > if (unshare_flags & ~(CLONE_THREAD|CLONE_FS|CLONE_NEWNS|CLONE_SIGHAND| > - CLONE_VM|CLONE_FILES|CLONE_SYSVSEM|CLONE_NEWUTS)) > + CLONE_VM|CLONE_FILES|CLONE_SYSVSEM| > + CLONE_NEWUTS|CLONE_NEWIPC)) > goto bad_unshare_out; > > if ((err = unshare_thread(unshare_flags)))
creator: ebiederm
date: 2006-06-12T18:01:48-00:00

Re: [PATCH 2/6] IPC namespace - utils
type: item
description: Eric W. Biederman wrote: > Cedric Le Goater <clg@fr.ibm.com> writes: > >> I've used the ipc namespace patchset in rc6-mm2. Thanks for putting this >> together, it works pretty well ! A few questions when we clone : >> >> * We should do something close to what exit_sem() already does to clear the >> sem_undo list from the task doing the clone() or unshare(). > > Possibly which case are you trying to prevent? task records a list of struct sem_undo each containing a semaphore id. When we unshare ipc namespace, we break the 'reference' between the semaphore id and the struct sem_array because the struct sem_array are cleared and freed in the new namespace. When the task exit, that inconstency could lead to unexpected results in exit_sem(), task locks, BUG_ON, etc. Nope ? >> * I don't like the idea of being able to unshare the ipc namespace and keep >> some shared memory from the previous ipc namespace mapped in the process mm. >> Should we forbid the unshare ? > > No. As long as the code handles that case properly we should be fine. what is the proper way to handle that case ? the current patchset is not protected : a process can be in one ipc namespace and use a shared segment from a previous ipc namespace. This situation is not desirable in a migration scenario. May be asking too much for the moment ... and I agree this can be fixed by the way namespaces are created. > As a general principle we should be able to keep things from other namespaces > open if we get them. The chroot or equivalent binary is the one that needs > to ensure these kinds of issues don't exist if we care. > > Speaking of we should put together a small test application probably similar > to chroot so people can access these features at least for testing. are you thinking about a command unshare()ing each namespace or some kind of create_nsproxy ? > Ack. For the unshare fix below. Could you resend this one separately with > patch in the subject so Andrew sees it and picks up? done. thanks, c.
creator: Cedric Le Goater
date: 2006-06-12T21:05:34-00:00

Re: [PATCH 2/6] IPC namespace - utils
type: item
description: Cedric Le Goater <clg@fr.ibm.com> writes: > Eric W. Biederman wrote: >> Cedric Le Goater <clg@fr.ibm.com> writes: >> >>> I've used the ipc namespace patchset in rc6-mm2. Thanks for putting this >>> together, it works pretty well ! A few questions when we clone : >>> >>> * We should do something close to what exit_sem() already does to clear the >>> sem_undo list from the task doing the clone() or unshare(). >> >> Possibly which case are you trying to prevent? > > task records a list of struct sem_undo each containing a semaphore id. When > we unshare ipc namespace, we break the 'reference' between the semaphore id > and the struct sem_array because the struct sem_array are cleared and freed > in the new namespace. When the task exit, that inconstency could lead to > unexpected results in exit_sem(), task locks, BUG_ON, etc. Nope ? Agreed. Hmm. I bet I didn't see this one earlier because it is specific to the unshare case. In this case I guess we should either deny the unshare or simply undo all of the semaphores. Because we will never be able to talk to them again. Thinking about this some more we need to unsharing the semaphore undo semantics when we create a new instances of the sysvipc namespace. Which means that until that piece is implemented we can't unshare the sysvipc namespace. But we clearly need the check in check_unshare_flags and the start of copy_process. >>> * I don't like the idea of being able to unshare the ipc namespace and keep >>> some shared memory from the previous ipc namespace mapped in the process mm. >>> Should we forbid the unshare ? >> >> No. As long as the code handles that case properly we should be fine. > > what is the proper way to handle that case ? the current patchset is not > protected : a process can be in one ipc namespace and use a shared segment > from a previous ipc namespace. This situation is not desirable in a > migration scenario. May be asking too much for the moment ... and I agree > this can be fixed by the way names…
creator: ebiederm
date: 2006-06-12T21:49:03-00:00

Re: [PATCH] IPC namespace
type: item
description: Kirill Korotaev <dev@openvz.org> writes: > The patches in this thread add IPC namespace functionality > additionally to already included in -mm tree UTS namespace. > > This patch set allows to unshare IPCs and have a private set > of IPC objects (sem, shm, msg) inside namespace. Basically, it is another > building block of containers functionality. > > Tested with LTP inside namespaces. > > Signed-Off-By: Pavel Emelianiov <xemul@openvz.org> > Signed-Off-By: Kirill Korotaev <dev@openvz.org> > > P.S. patches are against linux-2.6.17-rc6-mm1 Minor nit. These patches are not git-bisect safe. So if you have to apply them all to get a kernel that builds. Anyone trying to narrow down breakage is likely to land in the middle and hit a compile error. Eric
creator: ebiederm
date: 2006-06-13T02:44:52-00:00

Re: [PATCH] IPC namespace
type: item
description: >>P.S. patches are against linux-2.6.17-rc6-mm1 > > > Minor nit. These patches are not git-bisect safe. > So if you have to apply them all to get a kernel > that builds. > > Anyone trying to narrow down breakage is likely to land > in the middle and hit a compile error. Yes, these patches are hard to split (like with utsname...) I splitted them for easier review mostly... Kirill
creator: dev
date: 2006-06-13T16:41:36-00:00

Re: [PATCH] IPC namespace
type: item
description: Kirill Korotaev <dev@sw.ru> writes: >>>P.S. patches are against linux-2.6.17-rc6-mm1 >> Minor nit. These patches are not git-bisect safe. >> So if you have to apply them all to get a kernel >> that builds. >> Anyone trying to narrow down breakage is likely to land >> in the middle and hit a compile error. > > Yes, these patches are hard to split (like with utsname...) > I splitted them for easier review mostly... Reasonable. I'm slowly making my way through reviewing them. Eric
creator: ebiederm
date: 2006-06-13T17:01:21-00:00

Re: [PATCH 2/6] IPC namespace - utils
type: item
description: Eric W. Biederman wrote: >> task records a list of struct sem_undo each containing a semaphore id. When >> we unshare ipc namespace, we break the 'reference' between the semaphore id >> and the struct sem_array because the struct sem_array are cleared and freed >> in the new namespace. When the task exit, that inconstency could lead to >> unexpected results in exit_sem(), task locks, BUG_ON, etc. Nope ? > > Agreed. Hmm. I bet I didn't see this one earlier because it is specific > to the unshare case. In this case I guess we should either deny the unshare > or simply undo all of the semaphores. Because we will never be able to > talk to them again. So aren't we reaching the unshare() limits ? Shouldn't we be using the exec() principle for the sysvipc namespace ? clear it all and start from scratch. > Thinking about this some more we need to unsharing the semaphore undo semantics > when we create a new instances of the sysvipc namespace. Which means that > until that piece is implemented we can't unshare the sysvipc namespace. no big issue I think. exit_sem() does it already. it would end up coding the yet unsupported unshare_semundo(). > But we clearly need the check in check_unshare_flags and the start of copy_process. Yes. CLONE_SYSVSEM and CLONE_NEWIPC overlap in some ways. >>>> * I don't like the idea of being able to unshare the ipc namespace and keep >>>> some shared memory from the previous ipc namespace mapped in the process mm. >>>> Should we forbid the unshare ? >>> No. As long as the code handles that case properly we should be fine. >> what is the proper way to handle that case ? the current patchset is not >> protected : a process can be in one ipc namespace and use a shared segment >> from a previous ipc namespace. This situation is not desirable in a >> migration scenario. May be asking too much for the moment ... and I agree >> this can be fixed by the way namespaces are created. > > As long as the appropriate reference counting happens it shouldn't b…
creator: Cedric Le Goater
date: 2006-06-13T21:17:44-00:00

Re: [PATCH 2/6] IPC namespace - utils
type: item
description: >>Agreed. Hmm. I bet I didn't see this one earlier because it is specific >>to the unshare case. In this case I guess we should either deny the unshare >>or simply undo all of the semaphores. Because we will never be able to >>talk to them again. > > So aren't we reaching the unshare() limits ? Shouldn't we be using the > exec() principle for the sysvipc namespace ? clear it all and start from > scratch. there will be more such issues with more complex namespaces. That's why I proposed to use containers. Any way, right now, I don't think this should be urgently fixed as there are many other ways to crash the node when you are a root. The rule is simple - the process changing the namespace should be simple, w/o IPCs. For more complex namespaces (e.g. resource management namespaces) a fork() can be required after unshare(). >>Thinking about this some more we need to unsharing the semaphore undo semantics >>when we create a new instances of the sysvipc namespace. Which means that >>until that piece is implemented we can't unshare the sysvipc namespace. > > > no big issue I think. exit_sem() does it already. it would end up coding > the yet unsupported unshare_semundo(). do we need really it? my point is that while with IPCs it maybe quite easy since semundo code already exists it can be still very hard for other namespaces which do not track its objects associated with the task. BTW, do we have the same problem with shm? What will happen if the task having shm segment will change it's IPC namespace? Besides the possible crash/stability bugs there can be more interesting effects, for example: "user will be able to do operations on existing objects with new namespace limitations?"... Thanks, Kirill
creator: dev
date: 2006-06-14T11:14:49-00:00