On Fri, Jan 18, 2019 at 03:53:42PM +0100, Christian Brauner wrote:
> static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
> {
> + int ret;
> struct binderfs_info *info;
> - int ret = -ENOMEM;
> struct inode *inode = NULL;
> struct ipc_namespace *ipc_ns = current->nsproxy->ipc_ns;
>
> @@ -495,13 +495,14 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
> sb->s_op = &binderfs_super_ops;
> sb->s_time_gran = 1;
>
> - info = kzalloc(sizeof(struct binderfs_info), GFP_KERNEL);
> - if (!info)
> - goto err_without_dentry;
> + sb->s_fs_info = kzalloc(sizeof(struct binderfs_info), GFP_KERNEL);
> + if (!sb->s_fs_info)
> + return -ENOMEM;
> + info = sb->s_fs_info;
... and that's when you should grab ipcns reference and stick it into
info->ipc_ns, to match the logics in binderfs_kill_super().
Otherwise the failure above
> ret = binderfs_parse_mount_opts(data, &info->mount_opts);
> if (ret)
> - goto err_without_dentry;
> + return ret;
... or here leaves you with an ipcns leak.
Destructor does
if ->s_fs_info is non-NULL
release ->s_fs_info->ipc_ns
free ->s_fs_info
so constructor should not leave object in a state when ipcns is already
grabbed, but not stored in ->s_fs_info->ipc_ns (including the case of
allocation failure leaving it with NULL ->s_fs_info).
