All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Cc: "Dongliang Mu" <mudongliangabcd@gmail.com>,
	"Dongliang Mu" <dzm91@hust.edu.cn>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Todd Kjos" <tkjos@android.com>,
	"Martijn Coenen" <maco@android.com>,
	"Joel Fernandes" <joel@joelfernandes.org>,
	"Carlos Llamas" <cmllamas@google.com>,
	"Suren Baghdasaryan" <surenb@google.com>,
	"Kees Cook" <keescook@chromium.org>,
	syzkaller <syzkaller@googlegroups.com>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: [PATCH] binderfs: rework superblock destruction
Date: Wed, 17 Aug 2022 15:03:06 +0200	[thread overview]
Message-ID: <20220817130306.96978-1-brauner@kernel.org> (raw)
In-Reply-To: <YvzUS/7bd1mm6c/V@kroah.com>

From: Al Viro <viro@zeniv.linux.org.uk>

So far we relied on
.put_super = binderfs_put_super()
to destroy info we stashed in sb->s_fs_info. But the current implementation of
binderfs_fill_super() leads to a memory leak in the rare circumstance that
d_make_root() fails because ->put_super() is only called when sb->s_root is
initialized. Fix this by removing ->put_super() and simply do all that work in
binderfs_kill_super().

Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
I should note that I didn't have time to test this.
---
 drivers/android/binderfs.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 588d753a7a19..6d896f75aab6 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -340,22 +340,10 @@ static int binderfs_show_options(struct seq_file *seq, struct dentry *root)
 	return 0;
 }
 
-static void binderfs_put_super(struct super_block *sb)
-{
-	struct binderfs_info *info = sb->s_fs_info;
-
-	if (info && info->ipc_ns)
-		put_ipc_ns(info->ipc_ns);
-
-	kfree(info);
-	sb->s_fs_info = NULL;
-}
-
 static const struct super_operations binderfs_super_ops = {
 	.evict_inode    = binderfs_evict_inode,
 	.show_options	= binderfs_show_options,
 	.statfs         = simple_statfs,
-	.put_super	= binderfs_put_super,
 };
 
 static inline bool is_binderfs_control_device(const struct dentry *dentry)
@@ -785,11 +773,22 @@ static int binderfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
+static void binderfs_kill_super(struct super_block *sb)
+{
+	struct binderfs_info *info = sb->s_fs_info;
+
+	if (info && info->ipc_ns)
+		put_ipc_ns(info->ipc_ns);
+
+	kfree(info);
+	kill_litter_super(sb);
+}
+
 static struct file_system_type binder_fs_type = {
 	.name			= "binder",
 	.init_fs_context	= binderfs_init_fs_context,
 	.parameters		= binderfs_fs_parameters,
-	.kill_sb		= kill_litter_super,
+	.kill_sb		= binderfs_kill_super,
 	.fs_flags		= FS_USERNS_MOUNT,
 };
 
-- 
2.34.1


  reply	other threads:[~2022-08-17 13:03 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-12 13:21 [PATCH] drivers: binderfs: fix memory leak in binderfs_fill_super Dongliang Mu
2022-08-12 13:41 ` Christian Brauner
2022-08-12 13:48   ` Dongliang Mu
2022-08-12 14:18     ` Christian Brauner
2022-08-15  0:59       ` Dongliang Mu
2022-08-12 13:41 ` Greg Kroah-Hartman
2022-08-12 13:56   ` Dongliang Mu
2022-08-12 14:02     ` Dongliang Mu
2022-08-12 14:09     ` Greg Kroah-Hartman
2022-08-12 14:24       ` Christian Brauner
2022-08-12 14:32         ` Greg Kroah-Hartman
2022-08-15  1:46           ` Al Viro
2022-08-15  1:48             ` Al Viro
2022-08-15  8:47             ` Christian Brauner
2022-08-17 11:43               ` Greg Kroah-Hartman
2022-08-17 13:03                 ` Christian Brauner [this message]
2022-08-17 13:59                   ` [PATCH] binderfs: rework superblock destruction Al Viro
2022-08-17 14:01                     ` Christian Brauner
2022-08-17 14:19                       ` Al Viro
2022-08-17 14:32                         ` Al Viro
2022-08-17 15:05                           ` Christian Brauner
2022-08-17 14:51                         ` Christian Brauner
2022-08-17 15:21                           ` Al Viro
2022-08-17 15:24                             ` Christian Brauner

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=20220817130306.96978-1-brauner@kernel.org \
    --to=brauner@kernel.org \
    --cc=arve@android.com \
    --cc=cmllamas@google.com \
    --cc=dzm91@hust.edu.cn \
    --cc=gregkh@linuxfoundation.org \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maco@android.com \
    --cc=mudongliangabcd@gmail.com \
    --cc=surenb@google.com \
    --cc=syzkaller@googlegroups.com \
    --cc=tkjos@android.com \
    --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.