[Devel,RHEL7,COMMIT] ms/ipc/shm: handle removed segments gracefully in shm_mmap()

Submitted by Kirill Tkhai on Oct. 3, 2016, 2:49 p.m.

Details

Message ID 201610031449.u93EnZIQ010401@pro
State New
Series "replace remap_file_pages() syscall with emulation"
Headers show

Commit Message

Kirill Tkhai Oct. 3, 2016, 2:49 p.m.
The commit is pushed to "branch-rh7-3.10.0-327.36.1.vz7.18.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.36.1.vz7.18.3
------>
commit f5e3c5b8af4e24ec25c0e22062bf9a9461fb40f5
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Date:   Mon Oct 3 14:48:35 2016 +0000

    ms/ipc/shm: handle removed segments gracefully in shm_mmap()
    
    remap_file_pages(2) emulation can reach file which represents removed
    IPC ID as long as a memory segment is mapped.  It breaks expectations of
    IPC subsystem.
    
    Test case (rewritten to be more human readable, originally autogenerated
    by syzkaller[1]):
    
    	#define _GNU_SOURCE
    	#include <stdlib.h>
    	#include <sys/ipc.h>
    	#include <sys/mman.h>
    	#include <sys/shm.h>
    
    	#define PAGE_SIZE 4096
    
    	int main()
    	{
    		int id;
    		void *p;
    
    		id = shmget(IPC_PRIVATE, 3 * PAGE_SIZE, 0);
    		p = shmat(id, NULL, 0);
    		shmctl(id, IPC_RMID, NULL);
    		remap_file_pages(p, 3 * PAGE_SIZE, 0, 7, 0);
    
    	        return 0;
    	}
    
    The patch changes shm_mmap() and code around shm_lock() to propagate
    locking error back to caller of shm_mmap().
    
    [1] http://github.com/google/syzkaller
    
    Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
    Reported-by: Dmitry Vyukov <dvyukov@google.com>
    Cc: Davidlohr Bueso <dave@stgolabs.net>
    Cc: Manfred Spraul <manfred@colorfullife.com>
    Cc: <stable@vger.kernel.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    
    https://jira.sw.ru/browse/PSBM-52992
    
    (cherry picked from commit 1ac0b6dec656f3f78d1c3dd216fad84cb4d0a01e)
    Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
    Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 ipc/shm.c | 51 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/ipc/shm.c b/ipc/shm.c
index ccb99c0..5fbb2b7 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -155,9 +155,13 @@  static inline struct shmid_kernel *shm_lock(struct ipc_namespace *ns, int id)
 {
 	struct kern_ipc_perm *ipcp = ipc_lock(&shm_ids(ns), id);
 
+	/*
+	 * Callers of shm_lock() must validate the status of the returned ipc
+	 * object pointer (as returned by ipc_lock()), and error out as
+	 * appropriate.
+	 */
 	if (IS_ERR(ipcp))
 		return (struct shmid_kernel *)ipcp;
-
 	return container_of(ipcp, struct shmid_kernel, shm_perm);
 }
 
@@ -182,19 +186,32 @@  static inline void shm_rmid(struct ipc_namespace *ns, struct shmid_kernel *s)
 }
 
 
-/* This is called by fork, once for every shm attach. */
-static void shm_open(struct vm_area_struct *vma)
+static int __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(sfd->ns, sfd->id);
-	BUG_ON(IS_ERR(shp));
+	if (IS_ERR(shp))
+		return PTR_ERR(shp);
+
 	shp->shm_atim = get_seconds();
 	shp->shm_lprid = task_tgid_vnr(current);
 	shp->shm_nattch++;
 	shm_unlock(shp);
+	return 0;
+}
+
+/* This is called by fork, once for every shm attach. */
+static void shm_open(struct vm_area_struct *vma)
+{
+	int err = __shm_open(vma);
+	/*
+	 * We raced in the idr lookup or with shm_destroy().
+	 * Either way, the ID is busted.
+	 */
+	WARN_ON_ONCE(err);
 }
 
 /*
@@ -256,7 +273,14 @@  static void shm_close(struct vm_area_struct *vma)
 	down_write(&shm_ids(ns).rwsem);
 	/* remove from the list of attaches of the shm segment */
 	shp = shm_lock(ns, sfd->id);
-	BUG_ON(IS_ERR(shp));
+
+	/*
+	 * We raced in the idr lookup or with shm_destroy().
+	 * Either way, the ID is busted.
+	 */
+	if (WARN_ON_ONCE(IS_ERR(shp)))
+		goto done; /* no-op */
+
 	shp->shm_lprid = task_tgid_vnr(current);
 	shp->shm_dtim = get_seconds();
 	shp->shm_nattch--;
@@ -264,6 +288,7 @@  static void shm_close(struct vm_area_struct *vma)
 		shm_destroy(ns, shp);
 	else
 		shm_unlock(shp);
+done:
 	up_write(&shm_ids(ns).rwsem);
 }
 
@@ -384,17 +409,25 @@  static int shm_mmap(struct file * file, struct vm_area_struct * vma)
 	struct shm_file_data *sfd = shm_file_data(file);
 	int ret;
 
+	/*
+	 * In case of remap_file_pages() emulation, the file can represent
+	 * removed IPC ID: propogate shm_lock() error to caller.
+	 */
+	ret =__shm_open(vma);
+	if (ret)
+		return ret;
+
 	ret = sfd->file->f_op->mmap(sfd->file, vma);
-	if (ret != 0)
+	if (ret) {
+		shm_close(vma);
 		return ret;
+	}
 	sfd->vm_ops = vma->vm_ops;
 #ifdef CONFIG_MMU
 	BUG_ON(!sfd->vm_ops->fault);
 #endif
 	vma->vm_ops = &shm_vm_ops;
-	shm_open(vma);
-
-	return ret;
+	return 0;
 }
 
 static int shm_release(struct inode *ino, struct file *file)