[RFC][PATCH] ima: fix lockdep circular locking dependency

[Mimi Zohar <zohar@linux.vnet.ibm.com>]

The circular lockdep is caused by allocating the 'iint' for mmapped
files.  Originally when an 'iint' was allocated for every inode
in inode_alloc_security(), before the inode was accessible, no
locking was necessary.  Commits bc7d2a3e and 196f518 changed this
behavior and allocated the 'iint' on a per need basis, resulting in
the mmap_sem being taken before the i_mutex for mmapped files.

Possible unsafe locking scenario:
       CPU0                    CPU1
       ----                    ----
lock(&mm->mmap_sem);
                              lock(&sb->s_type->i_mutex_key);
                              lock(&mm->mmap_sem);
lock(&sb->s_type->i_mutex_key);

This patch adds a new hook ima_file_premmap() to pre-allocate the
iint, preventing the i_mutex being taken after the mmap_sem, and
defines a do_mmap() helper function do_mmap_with_sem().

Before making this sort of change throughout, perhaps someone sees
a better option?

thanks,

Mimi
---
 arch/x86/ia32/ia32_aout.c           |   24 ++++++++++--------------
 fs/binfmt_aout.c                    |   18 ++++++------------
 fs/binfmt_elf.c                     |    9 +++++----
 fs/binfmt_elf_fdpic.c               |    7 +++----
 fs/binfmt_flat.c                    |    5 ++---
 fs/binfmt_som.c                     |   21 +++++++++------------
 include/linux/ima.h                 |    6 ++++++
 include/linux/mm.h                  |    5 +++++
 ipc/shm.c                           |    4 +++-
 mm/mmap.c                           |   22 ++++++++++++++++++++++
 mm/nommu.c                          |    5 +++++
 security/integrity/ima/ima.h        |    2 +-
 security/integrity/ima/ima_main.c   |   27 +++++++++++++++++++++++++++
 security/integrity/ima/ima_policy.c |    2 ++
 14 files changed, 106 insertions(+), 51 deletions(-)

diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index fd84387..39fb63d 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -380,26 +380,22 @@ static int load_aout_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 			goto beyond_if;
 		}
 
-		down_write(&current->mm->mmap_sem);
-		error = do_mmap(bprm->file, N_TXTADDR(ex), ex.a_text,
+		error = do_mmap_with_sem(bprm->file, N_TXTADDR(ex), ex.a_text,
 				PROT_READ | PROT_EXEC,
 				MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE |
 				MAP_EXECUTABLE | MAP_32BIT,
-				fd_offset);
-		up_write(&current->mm->mmap_sem);
+				fd_offset, &current->mm->mmap_sem);
 
 		if (error != N_TXTADDR(ex)) {
 			send_sig(SIGKILL, current, 0);
 			return error;
 		}
 
-		down_write(&current->mm->mmap_sem);
-		error = do_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
+		error = do_mmap_with_sem(bprm->file, N_DATADDR(ex), ex.a_data,
 				PROT_READ | PROT_WRITE | PROT_EXEC,
 				MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE |
 				MAP_EXECUTABLE | MAP_32BIT,
-				fd_offset + ex.a_text);
-		up_write(&current->mm->mmap_sem);
+				fd_offset + ex.a_text, &current->mm->mmap_sem);
 		if (error != N_DATADDR(ex)) {
 			send_sig(SIGKILL, current, 0);
 			return error;
@@ -491,13 +487,13 @@ static int load_aout_library(struct file *file)
 		retval = 0;
 		goto out;
 	}
+	
 	/* Now use mmap to map the library into memory. */
-	down_write(&current->mm->mmap_sem);
-	error = do_mmap(file, start_addr, ex.a_text + ex.a_data,
-			PROT_READ | PROT_WRITE | PROT_EXEC,
-			MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_32BIT,
-			N_TXTOFF(ex));
-	up_write(&current->mm->mmap_sem);
+	error = do_mmap_with_sem(file, start_addr, ex.a_text + ex.a_data,
+				 PROT_READ | PROT_WRITE | PROT_EXEC,
+				 MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE
+				 | MAP_32BIT, N_TXTOFF(ex),
+				 &current->mm->mmap_sem);
 	retval = error;
 	if (error != start_addr)
 		goto out;
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index a6395bd..d8a4317 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -320,24 +320,20 @@ static int load_aout_binary(struct linux_binprm * bprm, struct pt_regs * regs)
 			goto beyond_if;
 		}
 
-		down_write(&current->mm->mmap_sem);
-		error = do_mmap(bprm->file, N_TXTADDR(ex), ex.a_text,
+		error = do_mmap_with_sem(bprm->file, N_TXTADDR(ex), ex.a_text,
 			PROT_READ | PROT_EXEC,
 			MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE,
-			fd_offset);
-		up_write(&current->mm->mmap_sem);
+			fd_offset, &current->mm->mmap_sem);
 
 		if (error != N_TXTADDR(ex)) {
 			send_sig(SIGKILL, current, 0);
 			return error;
 		}
 
-		down_write(&current->mm->mmap_sem);
- 		error = do_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
+ 		error = do_mmap_with_sem(bprm->file, N_DATADDR(ex), ex.a_data,
 				PROT_READ | PROT_WRITE | PROT_EXEC,
 				MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE,
-				fd_offset + ex.a_text);
-		up_write(&current->mm->mmap_sem);
+				fd_offset + ex.a_text, &current->mm->mmap_sem);
 		if (error != N_DATADDR(ex)) {
 			send_sig(SIGKILL, current, 0);
 			return error;
@@ -427,12 +423,10 @@ static int load_aout_library(struct file *file)
 		goto out;
 	}
 	/* Now use mmap to map the library into memory. */
-	down_write(&current->mm->mmap_sem);
-	error = do_mmap(file, start_addr, ex.a_text + ex.a_data,
+	error = do_mmap_with_sem(file, start_addr, ex.a_text + ex.a_data,
 			PROT_READ | PROT_WRITE | PROT_EXEC,
 			MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
-			N_TXTOFF(ex));
-	up_write(&current->mm->mmap_sem);
+			N_TXTOFF(ex), &current->mm->mmap_sem);
 	retval = error;
 	if (error != start_addr)
 		goto out;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 21ac5ee..9946b33 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -28,6 +28,7 @@
 #include <linux/highmem.h>
 #include <linux/pagemap.h>
 #include <linux/security.h>
+#include <linux/ima.h>
 #include <linux/random.h>
 #include <linux/elf.h>
 #include <linux/utsname.h>
@@ -330,6 +331,7 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
 	if (!size)
 		return addr;
 
+	ima_file_premmap(filep, prot);
 	down_write(&current->mm->mmap_sem);
 	/*
 	* total_size is the size of the ELF (interpreter) image.
@@ -1051,16 +1053,15 @@ static int load_elf_library(struct file *file)
 		eppnt++;
 
 	/* Now use mmap to map the library into memory. */
-	down_write(&current->mm->mmap_sem);
-	error = do_mmap(file,
+	error = do_mmap_with_sem(file,
 			ELF_PAGESTART(eppnt->p_vaddr),
 			(eppnt->p_filesz +
 			 ELF_PAGEOFFSET(eppnt->p_vaddr)),
 			PROT_READ | PROT_WRITE | PROT_EXEC,
 			MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
 			(eppnt->p_offset -
-			 ELF_PAGEOFFSET(eppnt->p_vaddr)));
-	up_write(&current->mm->mmap_sem);
+			 ELF_PAGEOFFSET(eppnt->p_vaddr)),
+			&current->mm->mmap_sem);
 	if (error != ELF_PAGESTART(eppnt->p_vaddr))
 		goto out_free_ph;
 
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 30745f4..b5baddf 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1097,10 +1097,9 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
 
 		/* create the mapping */
 		disp = phdr->p_vaddr & ~PAGE_MASK;
-		down_write(&mm->mmap_sem);
-		maddr = do_mmap(file, maddr, phdr->p_memsz + disp, prot, flags,
-				phdr->p_offset - disp);
-		up_write(&mm->mmap_sem);
+		maddr = do_mmap_with_sem(file, maddr, phdr->p_memsz + disp,
+					 prot, flags, phdr->p_offset - disp),
+					 &mm->mmap_sem);
 
 		kdebug("mmap[%d] <file> sz=%lx pr=%x fl=%x of=%lx --> %08lx",
 		       loop, phdr->p_memsz + disp, prot, flags,
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 1bffbe0..f617ef9 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -543,10 +543,9 @@ static int load_flat_file(struct linux_binprm * bprm,
 		 */
 		DBG_FLT("BINFMT_FLAT: ROM mapping of file (we hope)\n");
 
-		down_write(&current->mm->mmap_sem);
 		textpos = do_mmap(bprm->file, 0, text_len, PROT_READ|PROT_EXEC,
-				  MAP_PRIVATE|MAP_EXECUTABLE, 0);
-		up_write(&current->mm->mmap_sem);
+				  MAP_PRIVATE|MAP_EXECUTABLE, 0,
+				  &current->mm->mmap_sem);
 		if (!textpos || IS_ERR_VALUE(textpos)) {
 			if (!textpos)
 				textpos = (unsigned long) -ENOMEM;
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index cc8560f..0ff9b98 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -147,10 +147,9 @@ static int map_som_binary(struct file *file,
 	code_size = SOM_PAGEALIGN(hpuxhdr->exec_tsize);
 	current->mm->start_code = code_start;
 	current->mm->end_code = code_start + code_size;
-	down_write(&current->mm->mmap_sem);
-	retval = do_mmap(file, code_start, code_size, prot,
-			flags, SOM_PAGESTART(hpuxhdr->exec_tfile));
-	up_write(&current->mm->mmap_sem);
+	retval = do_mmap_with_sem(file, code_start, code_size, prot,
+			flags, SOM_PAGESTART(hpuxhdr->exec_tfile),
+			current->mm->mmap_sem);
 	if (retval < 0 && retval > -1024)
 		goto out;
 
@@ -158,20 +157,18 @@ static int map_som_binary(struct file *file,
 	data_size = SOM_PAGEALIGN(hpuxhdr->exec_dsize);
 	current->mm->start_data = data_start;
 	current->mm->end_data = bss_start = data_start + data_size;
-	down_write(&current->mm->mmap_sem);
-	retval = do_mmap(file, data_start, data_size,
+	retval = do_mmap_with_sem(file, data_start, data_size,
 			prot | PROT_WRITE, flags,
-			SOM_PAGESTART(hpuxhdr->exec_dfile));
-	up_write(&current->mm->mmap_sem);
+			SOM_PAGESTART(hpuxhdr->exec_dfile),
+			current->mm->mmap_sem);
 	if (retval < 0 && retval > -1024)
 		goto out;
 
 	som_brk = bss_start + SOM_PAGEALIGN(hpuxhdr->exec_bsize);
 	current->mm->start_brk = current->mm->brk = som_brk;
-	down_write(&current->mm->mmap_sem);
-	retval = do_mmap(NULL, bss_start, som_brk - bss_start,
-			prot | PROT_WRITE, MAP_FIXED | MAP_PRIVATE, 0);
-	up_write(&current->mm->mmap_sem);
+	retval = do_mmap_with_sem(NULL, bss_start, som_brk - bss_start,
+			prot | PROT_WRITE, MAP_FIXED | MAP_PRIVATE, 0,
+			&current->mm->mmap_sem);
 	if (retval > 0 || retval < -1024)
 		retval = 0;
 out:
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 6ac8e50..3862639 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -17,6 +17,7 @@ struct linux_binprm;
 extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask);
 extern void ima_file_free(struct file *file);
+extern int ima_file_premmap(struct file *file, unsigned long prot);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
 
 #else
@@ -35,6 +36,11 @@ static inline void ima_file_free(struct file *file)
 	return;
 }
 
+static inline int ima_file_premmap(struct file *file, unsigned long prot)
+{
+	return 0;
+}
+
 static inline int ima_file_mmap(struct file *file, unsigned long prot)
 {
 	return 0;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3dc3a8c..bf8da47 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1417,6 +1417,11 @@ out:
 	return ret;
 }
 
+extern unsigned long do_mmap_with_sem(struct file *file, unsigned long addr,
+	unsigned long len, unsigned long prot,
+	unsigned long flag, unsigned long offset,
+	struct rw_semaphore *mmap_sem);
+
 extern int do_munmap(struct mm_struct *, unsigned long, size_t);
 
 extern unsigned long do_brk(unsigned long, unsigned long);
diff --git a/ipc/shm.c b/ipc/shm.c
index 02ecf2c..e434c37 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -30,6 +30,7 @@
 #include <linux/mman.h>
 #include <linux/shmem_fs.h>
 #include <linux/security.h>
+#include <linux/ima.h>
 #include <linux/syscalls.h>
 #include <linux/audit.h>
 #include <linux/capability.h>
@@ -1042,7 +1043,8 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
 		    addr > current->mm->start_stack - size - PAGE_SIZE * 5)
 			goto invalid;
 	}
-		
+	
+	ima_file_premmap(file, prot);	
 	user_addr = do_mmap (file, addr, size, prot, flags, 0);
 	*raddr = user_addr;
 	err = 0;
diff --git a/mm/mmap.c b/mm/mmap.c
index eae90af..71ed785 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -20,6 +20,7 @@
 #include <linux/fs.h>
 #include <linux/personality.h>
 #include <linux/security.h>
+#include <linux/ima.h>
 #include <linux/hugetlb.h>
 #include <linux/profile.h>
 #include <linux/export.h>
@@ -1107,17 +1108,38 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 	}
 
 	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
+	retval = ima_file_premmap(file, prot);
+	if (retval)
+		goto err_out;
 
 	down_write(&current->mm->mmap_sem);
 	retval = do_mmap_pgoff(file, addr, len, prot, flags, pgoff);
 	up_write(&current->mm->mmap_sem);
 
+err_out:
 	if (file)
 		fput(file);
 out:
 	return retval;
 }
 
+unsigned long do_mmap_with_sem(struct file *file,
+	unsigned long addr,
+	unsigned long len, unsigned long prot,
+	unsigned long flag, unsigned long offset,
+	struct rw_semaphore *mmap_sem)
+{
+	unsigned long ret;
+
+	ima_file_premmap(file, prot);
+
+	down_write(mmap_sem);
+	ret = do_mmap(file, addr, len, prot, flag, offset);
+	up_write(mmap_sem);
+	return ret;
+}
+EXPORT_SYMBOL(do_mmap_with_sem);
+
 #ifdef __ARCH_WANT_SYS_OLD_MMAP
 struct mmap_arg_struct {
 	unsigned long addr;
diff --git a/mm/nommu.c b/mm/nommu.c
index 73419c5..bcb5b5e 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -27,6 +27,7 @@
 #include <linux/mount.h>
 #include <linux/personality.h>
 #include <linux/security.h>
+#include <linux/ima.h>
 #include <linux/syscalls.h>
 #include <linux/audit.h>
 
@@ -1485,11 +1486,15 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 	}
 
 	flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);
+	retval = ima_file_premmap(file, prot);
+	if (retval)
+		goto err_out;
 
 	down_write(&current->mm->mmap_sem);
 	retval = do_mmap_pgoff(file, addr, len, prot, flags, pgoff);
 	up_write(&current->mm->mmap_sem);
 
+err_out:
 	if (file)
 		fput(file);
 out:
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3ccf7ac..80819aa 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -114,7 +114,7 @@ struct integrity_iint_cache *integrity_iint_insert(struct inode *inode);
 struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
 
 /* IMA policy related functions */
-enum ima_hooks { FILE_CHECK = 1, FILE_MMAP, BPRM_CHECK };
+enum ima_hooks { FILE_CHECK = 1, FILE_PREMMAP, FILE_MMAP, BPRM_CHECK };
 
 int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask);
 void ima_init_policy(void);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1eff5cb..1df7ede 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -140,6 +140,9 @@ retry:
 		return rc;
 	}
 
+	if (function == FILE_PREMMAP)	/* defer to FILE_MMAP */
+		return 0;
+
 	mutex_lock(&iint->mutex);
 
 	rc = iint->flags & IMA_MEASURED ? 1 : 0;
@@ -153,6 +156,30 @@ out:
 	mutex_unlock(&iint->mutex);
 	return rc;
 }
+ 
+/**
+ * ima_file_premmap - based on policy allocate the 'iint'
+ * @file: pointer to the file to be measured (May be NULL)
+ * @prot: contains the protection that will be applied by the kernel.
+ *
+ * Based on the measurement policy, pre-allocate the iint before the
+ * mmap_sem is taken, but defer the actual measurement until
+ * security_file_mmap().
+ *
+ * (Pre-allocating the iint, prevents the i_mutex being taken after the
+ * mmap_sem.)
+ */
+int ima_file_premmap(struct file *file, unsigned long prot)
+{
+	int rc;
+
+	if (!file)
+		return 0;
+	if (prot & PROT_EXEC)
+		rc = process_measurement(file, file->f_dentry->d_name.name,
+					 MAY_EXEC, FILE_PREMMAP);
+	return 0;
+}
 
 /**
  * ima_file_mmap - based on policy, collect/store measurement.
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index d661afb..3a0e042 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -64,6 +64,8 @@ static struct ima_measure_rule_entry default_rules[] = {
 	{.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
 	{.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
 	{.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
+	{.action = MEASURE,.func = FILE_PREMMAP,.mask = MAY_EXEC,
+	 .flags = IMA_FUNC | IMA_MASK},
 	{.action = MEASURE,.func = FILE_MMAP,.mask = MAY_EXEC,
 	 .flags = IMA_FUNC | IMA_MASK},
 	{.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC,
-- 
1.7.6.4