Re: [PATCH RFC v2 01/18] xfs: fix the error unwind in xfs_open_devices()

On Tue 16-06-26 16:08:17, Christian Brauner wrote:
> Since the rt and log block devices are closed in xfs_free_buftarg() the
> buftarg owns the device file. The error unwind does not respect that:
> when the log buftarg allocation fails, out_free_rtdev_targ frees the rt
> buftarg - releasing rtdev_file - and then falls through to
> out_close_rtdev and releases it a second time.
> 
> The unwind also leaves mp->m_rtdev_targp and mp->m_ddev_targp pointing
> to the freed buftargs. The failed mount continues into
> deactivate_locked_super() -> xfs_kill_sb() -> xfs_mount_free(), which
> frees them again.
> 
> Clear the buftarg pointers once the unwind freed them and clear
> rtdev_file once the rt buftarg owns it, so nothing is released twice.
> 
> Reachable when a buftarg allocation fails after the data buftarg was
> set up: an I/O error in sync_blockdev() or an allocation failure in
> xfs_init_buftarg() while mounting with external rt and log devices.
> 
> Fixes: 41233576e9a4 ("xfs: close the RT and log block devices in xfs_free_buftarg")
> Signed-off-by: Christian Brauner (Amutable) <[email protected]>

Looks good to me. As a small nit I'd probably do rtdev_file = NULL just
after we've successfully allocated m_rtdev_targp but that's really minor.
Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

								Honza

> ---
>  fs/xfs/xfs_super.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index eac7f9503805..8531d526fc44 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -534,8 +534,11 @@ xfs_open_devices(
>   out_free_rtdev_targ:
>  	if (mp->m_rtdev_targp)
>  		xfs_free_buftarg(mp->m_rtdev_targp);
> +	mp->m_rtdev_targp = NULL;
> +	rtdev_file = NULL;	/* released by xfs_free_buftarg() */
>   out_free_ddev_targ:
>  	xfs_free_buftarg(mp->m_ddev_targp);
> +	mp->m_ddev_targp = NULL;
>   out_close_rtdev:
>  	 if (rtdev_file)
>  		bdev_fput(rtdev_file);
> 
> -- 
> 2.47.3
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR