Messages in this thread Patch in this message | | | Date | Mon, 15 Aug 2022 02:46:36 +0100 | From | Al Viro <> | Subject | Re: [PATCH] drivers: binderfs: fix memory leak in binderfs_fill_super |
| |
On Fri, Aug 12, 2022 at 04:32:28PM +0200, Greg Kroah-Hartman wrote:
> > It's a bit tricky to follow but d_make_root() always consumes the inode. > > On success via d_instantiate() and on failure via iput(). So when > > d_make_root() has been called the inode is off limits. And as soon as > > d_make_root() has returned successfully we're guaranteed that > > sb->s_fs_info is cleaned up if a ->put_super() method has been defined. > > Just fyi. > > Ah, thanks, that wasn't obvious at all. > > greg k-h
FWIW, I would rather provide a proper ->kill_sb() and gotten rid of all that stuff. The thing is, unlike ->put_super(), ->kill_sb() is called for *anything* that has gotten to foo_fill_super(). Usually allows to get rid of those "call all of or parts of foo_put_super() on failure exits" and associated bitrot...
Like this (completely untested):
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c index 588d753a7a19..c760f3129768 100644 --- a/drivers/android/binderfs.c +++ b/drivers/android/binderfs.c @@ -340,22 +340,21 @@ static int binderfs_show_options(struct seq_file *seq, struct dentry *root) return 0; } -static void binderfs_put_super(struct super_block *sb) +static void binderfs_kill_super(struct super_block *sb) { struct binderfs_info *info = sb->s_fs_info; + kill_litter_super(sb); 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) @@ -789,7 +788,7 @@ 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, };
| |