linux-kernel - [PATCH] kernfs: Improve lockdep annotation for files which implement mmap

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>] [day] [month] [year] [list]

Message-Id: <20191004161124.111376-1-bvanassche@acm.org>
Date:   Fri,  4 Oct 2019 09:11:24 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Tejun Heo <tj@...nel.org>, linux-kernel@...r.kernel.org,
        Bart Van Assche <bvanassche@....org>,
        Dave Jones <davej@...hat.com>
Subject: [PATCH] kernfs: Improve lockdep annotation for files which implement mmap

Instead of using one lockdep key for files that do not implement mmap
and another lockdep key for files that implement mmap, use a separate
lockdep key for each file that implements mmap.

This patch does not affect the size of struct kernfs_open_file if
CONFIG_LOCKDEP=n since in that case lock_class_key has size zero.

This patch is an improvement for commit 027a485d12e0 ("sysfs: use a
separate locking class for open files depending on mmap"; v3.13).

Cc: Dave Jones <davej@...hat.com>
Cc: Tejun Heo <tj@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Signed-off-by: Bart Van Assche <bvanassche@....org>
---
 fs/kernfs/file.c       | 34 +++++++++++++++++-----------------
 include/linux/kernfs.h |  1 +
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index e8c792b49616..a6980a8f9ffc 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -649,24 +649,17 @@ static int kernfs_fop_open(struct inode *inode, struct file *file)
 		goto err_out;
 
 	/*
-	 * The following is done to give a different lockdep key to
-	 * @of->mutex for files which implement mmap.  This is a rather
-	 * crude way to avoid false positive lockdep warning around
-	 * mm->mmap_sem - mmap nests @of->mutex under mm->mmap_sem and
-	 * reading /sys/block/sda/trace/act_mask grabs sr_mutex, under
-	 * which mm->mmap_sem nests, while holding @of->mutex.  As each
-	 * open file has a separate mutex, it's okay as long as those don't
-	 * happen on the same file.  At this point, we can't easily give
-	 * each file a separate locking class.  Let's differentiate on
-	 * whether the file has mmap or not for now.
-	 *
-	 * Both paths of the branch look the same.  They're supposed to
-	 * look that way and give @of->mutex different static lockdep keys.
+	 * Assign a unique lockdep key to @of->mutex for files which implement
+	 * mmap. This is necessary to avoid a false positive lockdep warning
+	 * around mm->mmap_sem - mmap nests @of->mutex under mm->mmap_sem and
+	 * reading /sys/block/sda/trace/act_mask grabs sr_mutex, under which
+	 * mm->mmap_sem nests, while holding @of->mutex.
 	 */
-	if (has_mmap)
-		mutex_init(&of->mutex);
-	else
-		mutex_init(&of->mutex);
+	mutex_init(&of->mutex);
+	if (has_mmap) {
+		lockdep_register_key(&of->mutex_key);
+		lockdep_set_class(&of->mutex, &of->mutex_key);
+	}
 
 	of->kn = kn;
 	of->file = file;
@@ -734,6 +727,9 @@ static int kernfs_fop_open(struct inode *inode, struct file *file)
 err_seq_release:
 	seq_release(inode, file);
 err_free:
+	mutex_destroy(&of->mutex);
+	if (has_mmap)
+		lockdep_unregister_key(&of->mutex_key);
 	kfree(of->prealloc_buf);
 	kfree(of);
 err_out:
@@ -769,6 +765,7 @@ static int kernfs_fop_release(struct inode *inode, struct file *filp)
 {
 	struct kernfs_node *kn = inode->i_private;
 	struct kernfs_open_file *of = kernfs_of(filp);
+	const bool has_mmap = kn->attr.ops->mmap != NULL;
 
 	if (kn->flags & KERNFS_HAS_RELEASE) {
 		mutex_lock(&kernfs_open_file_mutex);
@@ -778,6 +775,9 @@ static int kernfs_fop_release(struct inode *inode, struct file *filp)
 
 	kernfs_put_open_node(kn, of);
 	seq_release(inode, filp);
+	mutex_destroy(&of->mutex);
+	if (has_mmap)
+		lockdep_unregister_key(&of->mutex_key);
 	kfree(of->prealloc_buf);
 	kfree(of);
 
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 936b61bd504e..6680fae5f0f6 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -204,6 +204,7 @@ struct kernfs_open_file {
 	void			*priv;
 
 	/* private fields, do not use outside kernfs proper */
+	struct lock_class_key	mutex_key;
 	struct mutex		mutex;
 	struct mutex		prealloc_mutex;
 	int			event;
-- 
2.23.0.581.g78d2f28ef7-goog