linux-kernel - [PATCH] fs: Assert on module file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Oct 2016 13:35:52 -0700
From:   Calvin Owens <calvinowens@...com>
To:     Jeff Layton <jlayton@...chiereds.net>,
        "J. Bruce Fields" <bfields@...ldses.org>,
        Rusty Russell <rusty@...tcorp.com.au>
CC:     <linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <kernel-team@...com>, Calvin Owens <calvinowens@...com>
Subject: [PATCH] fs: Assert on module file_operations without an owner

Omitting the owner field in file_operations declared in modules is an
easy mistake to make, and can result in crashes when the module is
unloaded while userspace is poking the file.

This patch modifies fops_get() to WARN when it encounters a NULL owner,
since in this case it cannot take a reference on the containing module.

Signed-off-by: Calvin Owens <calvinowens@...com>
---
 include/linux/fs.h | 13 ++++++++++++-
 kernel/module.c    |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 901e25d..fafda9e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2081,10 +2081,21 @@ extern struct dentry *mount_pseudo(struct file_system_type *, char *,
 	unsigned long);
 
 /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
-#define fops_get(fops) \
+#define __fops_get(fops) \
 	(((fops) && try_module_get((fops)->owner) ? (fops) : NULL))
 #define fops_put(fops) \
 	do { if (fops) module_put((fops)->owner); } while(0)
+
+#define unowned_fmt "No fops owner at %p in [%s]\n"
+#define fops_unowned(fops) \
+	(is_module_address((unsigned long)(fops)) && !(fops)->owner)
+#define fops_modname(fops) \
+	__module_address((unsigned long)(fops))->name
+#define fops_warn_unowned(fops) \
+	WARN(fops_unowned(fops), unowned_fmt, (fops), fops_modname(fops))
+#define fops_get(fops) \
+	({ fops_warn_unowned(fops); __fops_get(fops); })
+
 /*
  * This one is to be used *ONLY* from ->open() instances.
  * fops must be non-NULL, pinned down *and* module dependencies
diff --git a/kernel/module.c b/kernel/module.c
index 529efae..4443727 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4181,6 +4181,7 @@ bool is_module_address(unsigned long addr)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(is_module_address);
 
 /*
  * __module_address - get the module which contains an address.
-- 
2.9.3