From bc56bba8f31bd99f350a5ebfd43d50f411b620c7 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 20 Feb 2007 13:57:53 -0800 Subject: [PATCH] shm: make sysv ipc shared memory use stacked files The current ipc shared memory code runs into several problems because it does not quite use files like the rest of the kernel. With the option of backing ipc shared memory with either hugetlbfs or ordinary shared memory the problems got worse. With the added support for ipc namespaces things behaved so unexpected that we now have several bad namespace reference counting bugs when using what appears at first glance to be a reasonable idiom. So to attack these problems and hopefully make the code more maintainable this patch simply uses the files provided by other parts of the kernel and builds it's own files out of them. The shm files are allocated in do_shmat and freed when their reference count drops to zero with their last unmap. The file and vm operations that we don't want to implement or we don't implement completely we just delegate to the operations of our backing file. This means that we now get an accurate shm_nattch count for we have a hugetlbfs inode for backing store, and the shm accounting of last attach and last detach time work as well. This means that getting a reference to the ipc namespace when we create the file and dropping the referenece in the release method is now safe and correct. This means we no longer need a special case for clearing VM_MAYWRITE as our file descriptor now only has write permissions when we have requested write access when calling shmat. Although VM_SHARED is now cleared as well which I believe is harmless and is mostly likely a minor bug fix. By using the same set of operations for both the hugetlb case and regular shared memory case shmdt is not simplified and made slightly more correct as now the test "vma->vm_ops == &shm_vm_ops" is 100% accurate in spotting all shared memory regions generated from sysvipc shared memory. Signed-off-by: Eric W. Biederman Cc: Michal Piotrowski Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/shm.c | 240 ++++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 155 insertions(+), 85 deletions(-) (limited to 'ipc') diff --git a/ipc/shm.c b/ipc/shm.c index 5bb617f6306..eb57e225430 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -37,11 +37,21 @@ #include #include #include +#include #include #include "util.h" +struct shm_file_data { + int id; + struct ipc_namespace *ns; + struct file *file; + const struct vm_operations_struct *vm_ops; +}; + +#define shm_file_data(file) (*((struct shm_file_data **)&(file)->private_data)) + static const struct file_operations shm_file_operations; static struct vm_operations_struct shm_vm_ops; @@ -60,8 +70,8 @@ static struct ipc_ids init_shm_ids; 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_open(struct vm_area_struct *vma); +static void shm_close(struct vm_area_struct *vma); 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); @@ -150,11 +160,14 @@ static inline int shm_addid(struct ipc_namespace *ns, struct shmid_kernel *shp) -static inline void shm_inc(struct ipc_namespace *ns, int id) +/* This is called by fork, once for every shm attach. */ +static void shm_open(struct vm_area_struct *vma) { + struct file *file = vma->vm_file; + struct shm_file_data *sfd = shm_file_data(file); struct shmid_kernel *shp; - shp = shm_lock(ns, id); + shp = shm_lock(sfd->ns, sfd->id); BUG_ON(!shp); shp->shm_atim = get_seconds(); shp->shm_lprid = current->tgid; @@ -162,15 +175,6 @@ static inline void shm_inc(struct ipc_namespace *ns, int id) shm_unlock(shp); } -#define shm_file_ns(file) (*((struct ipc_namespace **)&(file)->private_data)) - -/* This is called by fork, once for every shm attach. */ -static void shm_open(struct vm_area_struct *shmd) -{ - shm_inc(shm_file_ns(shmd->vm_file), - shmd->vm_file->f_path.dentry->d_inode->i_ino); -} - /* * shm_destroy - free the struct shmid_kernel * @@ -195,23 +199,21 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp) } /* - * remove the attach descriptor shmd. + * remove the attach descriptor vma. * free memory for segment if it is marked destroyed. * The descriptor has already been removed from the current->mm->mmap list * and will later be kfree()d. */ -static void shm_close (struct vm_area_struct *shmd) +static void shm_close(struct vm_area_struct *vma) { - struct file * file = shmd->vm_file; - int id = file->f_path.dentry->d_inode->i_ino; + struct file * file = vma->vm_file; + struct shm_file_data *sfd = shm_file_data(file); struct shmid_kernel *shp; - struct ipc_namespace *ns; - - ns = shm_file_ns(file); + struct ipc_namespace *ns = sfd->ns; mutex_lock(&shm_ids(ns).mutex); /* remove from the list of attaches of the shm segment */ - shp = shm_lock(ns, id); + shp = shm_lock(ns, sfd->id); BUG_ON(!shp); shp->shm_lprid = current->tgid; shp->shm_dtim = get_seconds(); @@ -224,46 +226,91 @@ static void shm_close (struct vm_area_struct *shmd) mutex_unlock(&shm_ids(ns).mutex); } +struct page *shm_nopage(struct vm_area_struct *vma, unsigned long address, + int *type) +{ + struct file *file = vma->vm_file; + struct shm_file_data *sfd = shm_file_data(file); + + return sfd->vm_ops->nopage(vma, address, type); +} + +#ifdef CONFIG_NUMA +int shm_set_policy(struct vm_area_struct *vma, struct mempolicy *new) +{ + struct file *file = vma->vm_file; + struct shm_file_data *sfd = shm_file_data(file); + int err = 0; + if (sfd->vm_ops->set_policy) + err = sfd->vm_ops->set_policy(vma, new); + return err; +} + +struct mempolicy *shm_get_policy(struct vm_area_struct *vma, unsigned long addr) +{ + struct file *file = vma->vm_file; + struct shm_file_data *sfd = shm_file_data(file); + struct mempolicy *pol = NULL; + + if (sfd->vm_ops->get_policy) + pol = sfd->vm_ops->get_policy(vma, addr); + else + pol = vma->vm_policy; + return pol; +} +#endif + static int shm_mmap(struct file * file, struct vm_area_struct * vma) { + struct shm_file_data *sfd = shm_file_data(file); int ret; - ret = shmem_mmap(file, vma); - if (ret == 0) { - vma->vm_ops = &shm_vm_ops; - if (!(vma->vm_flags & VM_WRITE)) - vma->vm_flags &= ~VM_MAYWRITE; - shm_inc(shm_file_ns(file), file->f_path.dentry->d_inode->i_ino); - } + ret = sfd->file->f_op->mmap(sfd->file, vma); + if (ret != 0) + return ret; + sfd->vm_ops = vma->vm_ops; + vma->vm_ops = &shm_vm_ops; + shm_open(vma); return ret; } static int shm_release(struct inode *ino, struct file *file) { - struct ipc_namespace *ns; + struct shm_file_data *sfd = shm_file_data(file); - ns = shm_file_ns(file); - put_ipc_ns(ns); - shm_file_ns(file) = NULL; + put_ipc_ns(sfd->ns); + shm_file_data(file) = NULL; + kfree(sfd); return 0; } +#ifndef CONFIG_MMU +static unsigned long shm_get_unmapped_area(struct file *file, + unsigned long addr, unsigned long len, unsigned long pgoff, + unsigned long flags) +{ + struct shm_file_data *sfd = shm_file_data(file); + return sfd->file->f_op->get_unmapped_area(sfd->file, addr, len, pgoff, + flags); +} +#else +#define shm_get_unmapped_area NULL +#endif + static const struct file_operations shm_file_operations = { .mmap = shm_mmap, .release = shm_release, -#ifndef CONFIG_MMU - .get_unmapped_area = shmem_get_unmapped_area, -#endif + .get_unmapped_area = shm_get_unmapped_area, }; static struct vm_operations_struct shm_vm_ops = { .open = shm_open, /* callback for a new vm-area open */ .close = shm_close, /* callback for when the vm-area is released */ - .nopage = shmem_nopage, -#if defined(CONFIG_NUMA) && defined(CONFIG_SHMEM) - .set_policy = shmem_set_policy, - .get_policy = shmem_get_policy, + .nopage = shm_nopage, +#if defined(CONFIG_NUMA) + .set_policy = shm_set_policy, + .get_policy = shm_get_policy, #endif }; @@ -330,13 +377,6 @@ static int newseg (struct ipc_namespace *ns, key_t key, int shmflg, size_t size) shp->shm_nattch = 0; shp->id = shm_buildid(ns, id, shp->shm_perm.seq); shp->shm_file = file; - file->f_path.dentry->d_inode->i_ino = shp->id; - - shm_file_ns(file) = get_ipc_ns(ns); - - /* Hugetlb ops would have already been assigned. */ - if (!(shmflg & SHM_HUGETLB)) - file->f_op = &shm_file_operations; ns->shm_tot += numpages; shm_unlock(shp); @@ -607,10 +647,7 @@ asmlinkage long sys_shmctl (int shmid, int cmd, struct shmid_ds __user *buf) tbuf.shm_ctime = shp->shm_ctim; tbuf.shm_cpid = shp->shm_cprid; tbuf.shm_lpid = shp->shm_lprid; - if (!is_file_hugepages(shp->shm_file)) - tbuf.shm_nattch = shp->shm_nattch; - else - tbuf.shm_nattch = file_count(shp->shm_file) - 1; + tbuf.shm_nattch = shp->shm_nattch; shm_unlock(shp); if(copy_shmid_to_user (buf, &tbuf, version)) err = -EFAULT; @@ -779,13 +816,16 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr) unsigned long flags; unsigned long prot; int acc_mode; - void *user_addr; + unsigned long user_addr; struct ipc_namespace *ns; + struct shm_file_data *sfd; + struct path path; + mode_t f_mode; - if (shmid < 0) { - err = -EINVAL; + err = -EINVAL; + if (shmid < 0) goto out; - } else if ((addr = (ulong)shmaddr)) { + else if ((addr = (ulong)shmaddr)) { if (addr & (SHMLBA-1)) { if (shmflg & SHM_RND) addr &= ~(SHMLBA-1); /* round down */ @@ -793,12 +833,12 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr) #ifndef __ARCH_FORCE_SHMLBA if (addr & ~PAGE_MASK) #endif - return -EINVAL; + goto out; } flags = MAP_SHARED | MAP_FIXED; } else { if ((shmflg & SHM_REMAP)) - return -EINVAL; + goto out; flags = MAP_SHARED; } @@ -806,9 +846,11 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr) if (shmflg & SHM_RDONLY) { prot = PROT_READ; acc_mode = S_IRUGO; + f_mode = FMODE_READ; } else { prot = PROT_READ | PROT_WRITE; acc_mode = S_IRUGO | S_IWUGO; + f_mode = FMODE_READ | FMODE_WRITE; } if (shmflg & SHM_EXEC) { prot |= PROT_EXEC; @@ -821,35 +863,50 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr) */ ns = current->nsproxy->ipc_ns; shp = shm_lock(ns, shmid); - if(shp == NULL) { - err = -EINVAL; + if(shp == NULL) goto out; - } + err = shm_checkid(ns, shp,shmid); - if (err) { - shm_unlock(shp); - goto out; - } - if (ipcperms(&shp->shm_perm, acc_mode)) { - shm_unlock(shp); - err = -EACCES; - goto out; - } + if (err) + goto out_unlock; + + err = -EACCES; + if (ipcperms(&shp->shm_perm, acc_mode)) + goto out_unlock; err = security_shm_shmat(shp, shmaddr, shmflg); - if (err) { - shm_unlock(shp); - return err; - } - - file = shp->shm_file; - size = i_size_read(file->f_path.dentry->d_inode); + if (err) + goto out_unlock; + + path.dentry = dget(shp->shm_file->f_path.dentry); + path.mnt = mntget(shp->shm_file->f_path.mnt); shp->shm_nattch++; + size = i_size_read(path.dentry->d_inode); shm_unlock(shp); + err = -ENOMEM; + sfd = kzalloc(sizeof(*sfd), GFP_KERNEL); + if (!sfd) + goto out_put_path; + + err = -ENOMEM; + file = get_empty_filp(); + if (!file) + goto out_free; + + file->f_op = &shm_file_operations; + file->private_data = sfd; + file->f_path = path; + file->f_mapping = shp->shm_file->f_mapping; + file->f_mode = f_mode; + sfd->id = shp->id; + sfd->ns = get_ipc_ns(ns); + sfd->file = shp->shm_file; + sfd->vm_ops = NULL; + down_write(¤t->mm->mmap_sem); if (addr && !(shmflg & SHM_REMAP)) { - user_addr = ERR_PTR(-EINVAL); + err = -EINVAL; if (find_vma_intersection(current->mm, addr, addr + size)) goto invalid; /* @@ -861,11 +918,17 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr) goto invalid; } - user_addr = (void*) do_mmap (file, addr, size, prot, flags, 0); - + user_addr = do_mmap (file, addr, size, prot, flags, 0); + *raddr = user_addr; + err = 0; + if (IS_ERR_VALUE(user_addr)) + err = (long)user_addr; invalid: up_write(¤t->mm->mmap_sem); + fput(file); + +out_nattch: mutex_lock(&shm_ids(ns).mutex); shp = shm_lock(ns, shmid); BUG_ON(!shp); @@ -877,12 +940,19 @@ invalid: shm_unlock(shp); mutex_unlock(&shm_ids(ns).mutex); - *raddr = (unsigned long) user_addr; - err = 0; - if (IS_ERR(user_addr)) - err = PTR_ERR(user_addr); out: return err; + +out_unlock: + shm_unlock(shp); + goto out; + +out_free: + kfree(sfd); +out_put_path: + dput(path.dentry); + mntput(path.mnt); + goto out_nattch; } asmlinkage long sys_shmat(int shmid, char __user *shmaddr, int shmflg) @@ -944,7 +1014,7 @@ asmlinkage long sys_shmdt(char __user *shmaddr) * a fragment created by mprotect() and/or munmap(), or it * otherwise it starts at this address with no hassles. */ - if ((vma->vm_ops == &shm_vm_ops || is_vm_hugetlb_page(vma)) && + if ((vma->vm_ops == &shm_vm_ops) && (vma->vm_start - addr)/PAGE_SIZE == vma->vm_pgoff) { @@ -973,7 +1043,7 @@ asmlinkage long sys_shmdt(char __user *shmaddr) next = vma->vm_next; /* finding a matching vma now does not alter retval */ - if ((vma->vm_ops == &shm_vm_ops || is_vm_hugetlb_page(vma)) && + if ((vma->vm_ops == &shm_vm_ops) && (vma->vm_start - addr)/PAGE_SIZE == vma->vm_pgoff) do_munmap(mm, vma->vm_start, vma->vm_end - vma->vm_start); @@ -1004,7 +1074,7 @@ static int sysvipc_shm_proc_show(struct seq_file *s, void *it) shp->shm_segsz, shp->shm_cprid, shp->shm_lprid, - is_file_hugepages(shp->shm_file) ? (file_count(shp->shm_file) - 1) : shp->shm_nattch, + shp->shm_nattch, shp->shm_perm.uid, shp->shm_perm.gid, shp->shm_perm.cuid, -- cgit v1.2.3 From de01bad2f5dec2977143aa242e7eba71d11a4363 Mon Sep 17 00:00:00 2001 From: Adrian Bunk Date: Wed, 28 Feb 2007 20:11:01 -0800 Subject: [PATCH] make ipc/shm.c:shm_nopage() static shm_nopage() can become static. Signed-off-by: Adrian Bunk Acked-by: Eric W. Biederman Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/shm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'ipc') diff --git a/ipc/shm.c b/ipc/shm.c index eb57e225430..3d0eb7940e9 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -226,8 +226,8 @@ static void shm_close(struct vm_area_struct *vma) mutex_unlock(&shm_ids(ns).mutex); } -struct page *shm_nopage(struct vm_area_struct *vma, unsigned long address, - int *type) +static struct page *shm_nopage(struct vm_area_struct *vma, + unsigned long address, int *type) { struct file *file = vma->vm_file; struct shm_file_data *sfd = shm_file_data(file); -- cgit v1.2.3 From 516dffdcd8827a40532798602830dfcfc672294c Mon Sep 17 00:00:00 2001 From: Adam Litke Date: Thu, 1 Mar 2007 15:46:08 -0800 Subject: [PATCH] Fix get_unmapped_area and fsync for hugetlb shm segments This patch provides the following hugetlb-related fixes to the recent stacked shm files changes: - Update is_file_hugepages() so it will reconize hugetlb shm segments. - get_unmapped_area must be called with the nested file struct to handle the sfd->file->f_ops->get_unmapped_area == NULL case. - The fsync f_op must be wrapped since it is specified in the hugetlbfs f_ops. This is based on proposed fixes from Eric Biederman that were debugged and tested by me. Without it, attempting to use hugetlb shared memory segments on powerpc (and likely ia64) will kill your box. Signed-off-by: Adam Litke Cc: Eric Biederman Cc: Andrew Morton Acked-by: William Irwin Signed-off-by: Linus Torvalds --- ipc/shm.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) (limited to 'ipc') diff --git a/ipc/shm.c b/ipc/shm.c index 3d0eb7940e9..4fefbad7096 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -285,21 +285,41 @@ static int shm_release(struct inode *ino, struct file *file) return 0; } -#ifndef CONFIG_MMU +static int shm_fsync(struct file *file, struct dentry *dentry, int datasync) +{ + int (*fsync) (struct file *, struct dentry *, int datasync); + struct shm_file_data *sfd = shm_file_data(file); + int ret = -EINVAL; + + fsync = sfd->file->f_op->fsync; + if (fsync) + ret = fsync(sfd->file, sfd->file->f_path.dentry, datasync); + return ret; +} + static unsigned long shm_get_unmapped_area(struct file *file, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags) { struct shm_file_data *sfd = shm_file_data(file); - return sfd->file->f_op->get_unmapped_area(sfd->file, addr, len, pgoff, - flags); + return get_unmapped_area(sfd->file, addr, len, pgoff, flags); +} + +int is_file_shm_hugepages(struct file *file) +{ + int ret = 0; + + if (file->f_op == &shm_file_operations) { + struct shm_file_data *sfd; + sfd = shm_file_data(file); + ret = is_file_hugepages(sfd->file); + } + return ret; } -#else -#define shm_get_unmapped_area NULL -#endif static const struct file_operations shm_file_operations = { .mmap = shm_mmap, + .fsync = shm_fsync, .release = shm_release, .get_unmapped_area = shm_get_unmapped_area, }; -- cgit v1.2.3 From 7a434814c7a6500b08bf4419ba8712b152d08d08 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Tue, 6 Mar 2007 01:42:09 -0800 Subject: [PATCH] mqueue: nested locking annotation Fix http://bugzilla.kernel.org/show_bug.cgi?id=8130 Signed-off-by: Peter Zijlstra Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/mqueue.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'ipc') diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 0b5ecbe5f04..554ac368be7 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -731,7 +731,8 @@ asmlinkage long sys_mq_unlink(const char __user *u_name) if (IS_ERR(name)) return PTR_ERR(name); - mutex_lock(&mqueue_mnt->mnt_root->d_inode->i_mutex); + mutex_lock_nested(&mqueue_mnt->mnt_root->d_inode->i_mutex, + I_MUTEX_PARENT); dentry = lookup_one_len(name, mqueue_mnt->mnt_root, strlen(name)); if (IS_ERR(dentry)) { err = PTR_ERR(dentry); -- cgit v1.2.3 From a28d193cbf01375974683c13e99a52ef489e5eb0 Mon Sep 17 00:00:00 2001 From: "Serge E. Hallyn" Date: Mon, 26 Mar 2007 21:32:31 -0800 Subject: [PATCH] ipcns: fix !CONFIG_IPC_NS behavior When CONFIG_IPC_NS=n, clone(CLONE_NEWIPC) claims success, but did not actually clone a new IPC namespace. Fix this to return -EINVAL so the caller knows his request was denied. Signed-off-by: Serge E. Hallyn Cc: "Eric W. Biederman" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/util.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'ipc') diff --git a/ipc/util.c b/ipc/util.c index 08a647965b9..0b652387d16 100644 --- a/ipc/util.c +++ b/ipc/util.c @@ -144,6 +144,13 @@ void free_ipc_ns(struct kref *kref) shm_exit_ns(ns); kfree(ns); } +#else +int copy_ipcs(unsigned long flags, struct task_struct *tsk) +{ + if (flags & CLONE_NEWIPC) + return -EINVAL; + return 0; +} #endif /** -- cgit v1.2.3