Bug#796036: linux-image-3.16.0-4-amd64: concurrent msync triggers NULL pointer dereference

[Ben Hutchings <ben@decadent.org.uk>]

Control: tag -1 security patch

On Tue, 2015-08-18 at 20:05 +0200, Xavier Chantry wrote:
> Package: src:linux
> Version: 3.16.7-ckt11-1
> Severity: important
> 
> Using Debian 3.16.7-ckt4-3 and a simple test case, we were able to reproduce a
> kernel bug in msync system call.
[...]

I can reproduce this too.  I also found a similar problem with
madvise(..., MADV_REMOVE).  The attached patch (against
3.16.7-ckt11-1+deb8u3) should fix them both.

Ben.

-- 
Ben Hutchings
The most exhausting thing in life is being insincere. - Anne Morrow Lindberg

From: Ben Hutchings <ben@decadent.org.uk>
Date: Thu, 10 Sep 2015 02:19:59 +0100
Subject: aufs3: mmap: Fix races in madvise_remove() and sys_msync()
Bug-Debian: https://bugs.debian.org/796036

In madvise_remove() and sys_msync() we drop the mmap_sem before
dropping references to the mapped file(s).  As soon as we drop the
mmap_sem, the vma we got them from might be destroyed by another
thread, so calling vma_do_fput() is a possible use-after-free.

Introduce __vma_do_fput() which takes the two file pointers
(vm_file and vm_prfile) instead of a vma.  Use it in the above
two functions.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1193,6 +1193,7 @@ extern struct file *vma_do_pr_or_file(st
 				      int);
 extern void vma_do_get_file(struct vm_area_struct *, const char[], int);
 extern void vma_do_fput(struct vm_area_struct *, const char[], int);
+extern void __vma_do_fput(struct file *, struct file *, const char[], int);
 
 #define vma_file_update_time(vma)	vma_do_file_update_time(vma, __func__, \
 								__LINE__)
@@ -1200,6 +1201,7 @@ extern void vma_do_fput(struct vm_area_s
 							  __LINE__)
 #define vma_get_file(vma)		vma_do_get_file(vma, __func__, __LINE__)
 #define vma_fput(vma)			vma_do_fput(vma, __func__, __LINE__)
+#define __vma_fput(f, pr)		__vma_do_fput(f, pr, __func__, __LINE__)
 #else
 extern struct file *vmr_do_pr_or_file(struct vm_region *, const char[], int);
 extern void vmr_do_fput(struct vm_region *, const char[], int);
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -302,7 +302,7 @@ static long madvise_remove(struct vm_are
 {
 	loff_t offset;
 	int error;
-	struct file *f;
+	struct file *f, *pr;
 
 	*prev = NULL;	/* tell sys_madvise we drop mmap_sem */
 
@@ -310,6 +310,7 @@ static long madvise_remove(struct vm_are
 		return -EINVAL;
 
 	f = vma->vm_file;
+	pr = vma->vm_prfile;
 
 	if (!f || !f->f_mapping || !f->f_mapping->host) {
 			return -EINVAL;
@@ -332,7 +333,7 @@ static long madvise_remove(struct vm_are
 	error = do_fallocate(f,
 				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 				offset, end - start);
-	vma_fput(vma);
+	__vma_fput(f, pr);
 	down_read(&current->mm->mmap_sem);
 	return error;
 }
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -57,7 +57,7 @@ SYSCALL_DEFINE3(msync, unsigned long, st
 	down_read(&mm->mmap_sem);
 	vma = find_vma(mm, start);
 	for (;;) {
-		struct file *file;
+		struct file *file, *prfile;
 		loff_t fstart, fend;
 
 		/* Still start < end. */
@@ -78,6 +78,7 @@ SYSCALL_DEFINE3(msync, unsigned long, st
 			goto out_unlock;
 		}
 		file = vma->vm_file;
+		prfile = vma->vm_prfile;
 		fstart = (start - vma->vm_start) +
 			 ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
 		fend = fstart + (min(end, vma->vm_end) - start) - 1;
@@ -90,7 +91,7 @@ SYSCALL_DEFINE3(msync, unsigned long, st
 				error = vfs_fsync(file, 1);
 			else
 				error = vfs_fsync_range(file, fstart, fend, 1);
-			vma_fput(vma);
+			__vma_fput(file, prfile);
 			if (error || start >= end)
 				goto out;
 			down_read(&mm->mmap_sem);
--- a/mm/prfile.c
+++ b/mm/prfile.c
@@ -57,8 +57,11 @@ void vma_do_get_file(struct vm_area_stru
 
 void vma_do_fput(struct vm_area_struct *vma, const char func[], int line)
 {
-	struct file *f = vma->vm_file, *pr = vma->vm_prfile;
+	__vma_do_fput(vma->vm_file, vma->vm_prfile, func, line);
+}
 
+void __vma_do_fput(struct file *f, struct file *pr, const char func[], int line)
+{
 	prfile_trace(f, pr, func, line, __func__);
 	fput(f);
 	if (f && pr)

Attachment: signature.asc
Description: This is a digitally signed message part