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
