Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932762AbcJGUgF (ORCPT ); Fri, 7 Oct 2016 16:36:05 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:52603 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753254AbcJGUf5 (ORCPT ); Fri, 7 Oct 2016 16:35:57 -0400 From: Calvin Owens To: Jeff Layton , "J. Bruce Fields" , Rusty Russell CC: , , , Calvin Owens Subject: [PATCH] fs: Assert on module file_operations without an owner Date: Fri, 7 Oct 2016 13:35:52 -0700 Message-ID: <48414ef29337b54e2a842bd841f73f01ab74ebe7.1475872278.git.calvinowens@fb.com> X-Mailer: git-send-email 2.9.3 X-FB-Internal: Safe MIME-Version: 1.0 Content-Type: text/plain X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-10-07_09:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1878 Lines: 54 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 --- 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