Re: [PATCH 3/5] binderfs: rework binderfs_fill_super()

[Al Viro <viro@zeniv.linux.org.uk>]

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).