All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mimi Zohar <zohar@linux.ibm.com>
To: linux-integrity@vger.kernel.org
Cc: Mimi Zohar <zohar@linux.ibm.com>,
	Eric Biggers <ebiggers@kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Subject: [RFC PATCH] ima: fix ima_file_mmap circular locking dependency
Date: Fri, 12 Jul 2019 16:41:37 -0400	[thread overview]
Message-ID: <1562964097-8578-1-git-send-email-zohar@linux.ibm.com> (raw)

The LSM security_mmap_file hook is called before the mmap_sem is taken.
This results in IMA taking the i_mutex before the mmap_sem, yet the
normal locking order is mmap_sem, i_mutex.

To resolve this problem, rename and call ima_mmap_file() after taking
the mmap_sem.

Reported-by: syzbot+5ab61747675a87ea359d@syzkaller.appspotmail.com
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 include/linux/ima.h               | 4 ++--
 ipc/shm.c                         | 5 +++++
 mm/util.c                         | 8 ++++++++
 security/integrity/ima/ima_main.c | 2 +-
 security/security.c               | 8 ++------
 5 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 71796a0959d9..10adb38e0e43 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -17,7 +17,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask);
 extern void ima_post_create_tmpfile(struct inode *inode);
 extern void ima_file_free(struct file *file);
-extern int ima_file_mmap(struct file *file, unsigned long prot);
+extern int ima_mmap_file(struct file *file, unsigned long prot);
 extern int ima_load_data(enum kernel_load_data_id id);
 extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
@@ -65,7 +65,7 @@ static inline void ima_file_free(struct file *file)
 	return;
 }
 
-static inline int ima_file_mmap(struct file *file, unsigned long prot)
+static inline int ima_mmap_file(struct file *file, unsigned long prot)
 {
 	return 0;
 }
diff --git a/ipc/shm.c b/ipc/shm.c
index ce1ca9f7c6e9..a712c7d426f0 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -34,6 +34,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>
@@ -1549,6 +1550,10 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
 		goto out_fput;
 	}
 
+	err = ima_mmap_file(file, prot);
+	if (err)
+		goto out_fput;
+
 	if (addr && !(shmflg & SHM_REMAP)) {
 		err = -EINVAL;
 		if (addr + size < addr)
diff --git a/mm/util.c b/mm/util.c
index 9834c4ab7d8e..dbf2c15caacd 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -9,6 +9,7 @@
 #include <linux/sched/mm.h>
 #include <linux/sched/task_stack.h>
 #include <linux/security.h>
+#include <linux/ima.h>
 #include <linux/swap.h>
 #include <linux/swapops.h>
 #include <linux/mman.h>
@@ -360,6 +361,13 @@ unsigned long vm_mmap_pgoff(struct file *file, unsigned long addr,
 	if (!ret) {
 		if (down_write_killable(&mm->mmap_sem))
 			return -EINTR;
+
+		ret = ima_mmap_file(file, prot);
+		if (ret) {
+			up_write(&mm->mmap_sem);
+			return ret;
+		}
+
 		ret = do_mmap_pgoff(file, addr, len, prot, flag, pgoff,
 				    &populate, &uf);
 		up_write(&mm->mmap_sem);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 45d9ece88668..14678665cdc8 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -380,7 +380,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
  * On success return 0.  On integrity appraisal error, assuming the file
  * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
  */
-int ima_file_mmap(struct file *file, unsigned long prot)
+int ima_mmap_file(struct file *file, unsigned long prot)
 {
 	u32 secid;
 
diff --git a/security/security.c b/security/security.c
index a749d884faec..e324c425e466 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1410,12 +1410,8 @@ static inline unsigned long mmap_prot(struct file *file, unsigned long prot)
 int security_mmap_file(struct file *file, unsigned long prot,
 			unsigned long flags)
 {
-	int ret;
-	ret = call_int_hook(mmap_file, 0, file, prot,
-					mmap_prot(file, prot), flags);
-	if (ret)
-		return ret;
-	return ima_file_mmap(file, prot);
+	return call_int_hook(mmap_file, 0, file, prot,
+			     mmap_prot(file, prot), flags);
 }
 
 int security_mmap_addr(unsigned long addr)
-- 
2.7.5


             reply	other threads:[~2019-07-12 20:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12 20:41 Mimi Zohar [this message]
2019-07-12 23:13 ` [RFC PATCH] ima: fix ima_file_mmap circular locking dependency Eric Biggers
2019-07-14 16:48   ` Mimi Zohar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1562964097-8578-1-git-send-email-zohar@linux.ibm.com \
    --to=zohar@linux.ibm.com \
    --cc=ebiggers@kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.