| Message ID | d4186215561658577a9622bf111c79909f0521c6.1718883560.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: qgroup: fix quota root leak after quota disable failure | expand |
On Thu, Jun 20, 2024 at 12:41:09PM +0100, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > If during the quota disable we fail when cleaning the quota tree or when > deleting the root from the root tree, we jump to the 'out' label without > ever dropping the reference on the quota root, resulting in a leak of the > root since fs_info->quota_root is no longer pointing to the root (we have > set it to NULL just before those steps). > > Fix this by always doing a btrfs_put_root() call under the 'out' label. > This is a problem that exists since qgroups were first added in 2012 by > commit bed92eae26cc ("Btrfs: qgroup implementation and prototypes"), but > back then we missed a kfree on the quota root and free_extent_buffer() > calls on its root and commit root nodes, since back then roots were not > yet reference counted. > Reviewed-by: Boris Burkov <boris@bur.io> > Signed-off-by: Filipe Manana <fdmanana@suse.com> > --- > fs/btrfs/qgroup.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index 3edbe5bb19c6..d89240512796 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1346,7 +1346,7 @@ static int flush_reservations(struct btrfs_fs_info *fs_info) > > int btrfs_quota_disable(struct btrfs_fs_info *fs_info) > { > - struct btrfs_root *quota_root; > + struct btrfs_root *quota_root = NULL; > struct btrfs_trans_handle *trans = NULL; > int ret = 0; > > @@ -1445,9 +1445,9 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info) > quota_root->node, 0, 1); > if (ret < 0) > btrfs_abort_transaction(trans, ret); > - btrfs_put_root(quota_root); > > out: > + btrfs_put_root(quota_root); > mutex_unlock(&fs_info->qgroup_ioctl_lock); > if (ret && trans) > btrfs_end_transaction(trans); > -- > 2.43.0 >
在 2024/6/20 21:11, fdmanana@kernel.org 写道: > From: Filipe Manana <fdmanana@suse.com> > > If during the quota disable we fail when cleaning the quota tree or when > deleting the root from the root tree, we jump to the 'out' label without > ever dropping the reference on the quota root, resulting in a leak of the > root since fs_info->quota_root is no longer pointing to the root (we have > set it to NULL just before those steps). > > Fix this by always doing a btrfs_put_root() call under the 'out' label. > This is a problem that exists since qgroups were first added in 2012 by > commit bed92eae26cc ("Btrfs: qgroup implementation and prototypes"), but > back then we missed a kfree on the quota root and free_extent_buffer() > calls on its root and commit root nodes, since back then roots were not > yet reference counted. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/qgroup.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index 3edbe5bb19c6..d89240512796 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1346,7 +1346,7 @@ static int flush_reservations(struct btrfs_fs_info *fs_info) > > int btrfs_quota_disable(struct btrfs_fs_info *fs_info) > { > - struct btrfs_root *quota_root; > + struct btrfs_root *quota_root = NULL; > struct btrfs_trans_handle *trans = NULL; > int ret = 0; > > @@ -1445,9 +1445,9 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info) > quota_root->node, 0, 1); > if (ret < 0) > btrfs_abort_transaction(trans, ret); > - btrfs_put_root(quota_root); > > out: > + btrfs_put_root(quota_root); > mutex_unlock(&fs_info->qgroup_ioctl_lock); > if (ret && trans) > btrfs_end_transaction(trans);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index 3edbe5bb19c6..d89240512796 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1346,7 +1346,7 @@ static int flush_reservations(struct btrfs_fs_info *fs_info) int btrfs_quota_disable(struct btrfs_fs_info *fs_info) { - struct btrfs_root *quota_root; + struct btrfs_root *quota_root = NULL; struct btrfs_trans_handle *trans = NULL; int ret = 0; @@ -1445,9 +1445,9 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info) quota_root->node, 0, 1); if (ret < 0) btrfs_abort_transaction(trans, ret); - btrfs_put_root(quota_root); out: + btrfs_put_root(quota_root); mutex_unlock(&fs_info->qgroup_ioctl_lock); if (ret && trans) btrfs_end_transaction(trans);