| Message ID | b6c30d9569d56552e38dc6bd305c6eb6578f3782.1639162814.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fix double free of anon_dev after failure to create subvolume | expand |
On 2021/12/11 03:02, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > When creating a subvolume, at create_subvol(), we allocate an anonymous > device and later call btrfs_get_new_fs_root(), which in turn just calls > btrfs_get_root_ref(). There we call btrfs_init_fs_root() which assigns > the anonymous device to the root, but if after that call there's an error, > when we jump to 'fail' label, we call btrfs_put_root(), which frees the > anonymous device and then returns an error that is propagated back to > create_subvol(). Than create_subvol() frees the anonymous device again. > > When this happens, if the anonymous device was not reallocated after > the first time it was freed with btrfs_put_root(), we get a kernel > message like the following: > > (...) > [13950.282466] BTRFS: error (device dm-0) in create_subvol:663: errno=-5 IO failure > [13950.283027] ida_free called for id=65 which is not allocated. > [13950.285974] BTRFS info (device dm-0): forced readonly > (...) > > If the anonymous device gets reallocated by another btrfs filesystem > or any other kernel subsystem, then bad things can happen. > > So fix this by setting the root's anonymous device to 0 at > btrfs_get_root_ref(), before we call btrfs_put_root(), if an error > happened. > > Fixes: 2dfb1e43f57dd3 ("btrfs: preallocate anon block device at first phase of snapshot creation") > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/disk-io.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 5c598e124c25..fc7dd5109806 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -1826,6 +1826,14 @@ static struct btrfs_root *btrfs_get_root_ref(struct btrfs_fs_info *fs_info, > } > return root; > fail: > + /* > + * If our caller provided us an anonymous device, then it's his > + * responsability to free it in case we fail. So we have to set our > + * root's anon_dev to 0 to avoid a double free, once by btrfs_put_root() > + * and once again by our caller. > + */ > + if (anon_dev) > + root->anon_dev = 0; > btrfs_put_root(root); > return ERR_PTR(ret); > }
On Fri, Dec 10, 2021 at 07:02:18PM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > When creating a subvolume, at create_subvol(), we allocate an anonymous > device and later call btrfs_get_new_fs_root(), which in turn just calls > btrfs_get_root_ref(). There we call btrfs_init_fs_root() which assigns > the anonymous device to the root, but if after that call there's an error, > when we jump to 'fail' label, we call btrfs_put_root(), which frees the > anonymous device and then returns an error that is propagated back to > create_subvol(). Than create_subvol() frees the anonymous device again. > > When this happens, if the anonymous device was not reallocated after > the first time it was freed with btrfs_put_root(), we get a kernel > message like the following: > > (...) > [13950.282466] BTRFS: error (device dm-0) in create_subvol:663: errno=-5 IO failure > [13950.283027] ida_free called for id=65 which is not allocated. > [13950.285974] BTRFS info (device dm-0): forced readonly > (...) > > If the anonymous device gets reallocated by another btrfs filesystem > or any other kernel subsystem, then bad things can happen. > > So fix this by setting the root's anonymous device to 0 at > btrfs_get_root_ref(), before we call btrfs_put_root(), if an error > happened. > > Fixes: 2dfb1e43f57dd3 ("btrfs: preallocate anon block device at first phase of snapshot creation") > Signed-off-by: Filipe Manana <fdmanana@suse.com> Added to misc-next, thanks.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 5c598e124c25..fc7dd5109806 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -1826,6 +1826,14 @@ static struct btrfs_root *btrfs_get_root_ref(struct btrfs_fs_info *fs_info, } return root; fail: + /* + * If our caller provided us an anonymous device, then it's his + * responsability to free it in case we fail. So we have to set our + * root's anon_dev to 0 to avoid a double free, once by btrfs_put_root() + * and once again by our caller. + */ + if (anon_dev) + root->anon_dev = 0; btrfs_put_root(root); return ERR_PTR(ret); }