| Message ID | 2431d473c04bede4387c081007d532758fcd2f28.1699301753.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fix qgroup record leaks when using simple quotas | expand |
On 2023/11/7 06:47, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > When using simple quotas we are not supposed to allocate qgroup records > when adding delayed references. However we allocate them if either mode > of quotas is enabled (the new simple one or the old one), but then we > never free them because running the accounting, which frees the records, > is only run when using the old quotas (at btrfs_qgroup_account_extents()), > resulting in a memory leak of the records allocated when adding delayed > references. > > Fix this by allocating the records only if the old quotas mode is enabled. > Also fix btrfs_qgroup_trace_extent_nolock() to return 1 if the old quotas > mode is not enabled - meaning the caller has to free the record. > > Fixes: 182940f4f4db ("btrfs: qgroup: add new quota mode for simple quotas") > Reported-by: syzbot+d3ddc6dcc6386dea398b@syzkaller.appspotmail.com > Link: https://lore.kernel.org/linux-btrfs/00000000000004769106097f9a34@google.com/ > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/delayed-ref.c | 4 ++-- > fs/btrfs/qgroup.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c > index 9223934d95f4..891ea2fa263c 100644 > --- a/fs/btrfs/delayed-ref.c > +++ b/fs/btrfs/delayed-ref.c > @@ -1041,7 +1041,7 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans, > return -ENOMEM; > } > > - if (btrfs_qgroup_enabled(fs_info) && !generic_ref->skip_qgroup) { > + if (btrfs_qgroup_full_accounting(fs_info) && !generic_ref->skip_qgroup) { > record = kzalloc(sizeof(*record), GFP_NOFS); > if (!record) { > kmem_cache_free(btrfs_delayed_tree_ref_cachep, ref); > @@ -1144,7 +1144,7 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans, > return -ENOMEM; > } > > - if (btrfs_qgroup_enabled(fs_info) && !generic_ref->skip_qgroup) { > + if (btrfs_qgroup_full_accounting(fs_info) && !generic_ref->skip_qgroup) { > record = kzalloc(sizeof(*record), GFP_NOFS); > if (!record) { > kmem_cache_free(btrfs_delayed_data_ref_cachep, ref); > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index e48eba7e9379..ce446d9d7f23 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1888,7 +1888,7 @@ int btrfs_qgroup_trace_extent_nolock(struct btrfs_fs_info *fs_info, > u64 bytenr = record->bytenr; > > if (!btrfs_qgroup_full_accounting(fs_info)) > - return 0; > + return 1; > > lockdep_assert_held(&delayed_refs->lock); > trace_btrfs_qgroup_trace_extent(fs_info, record);
On Mon, Nov 06, 2023 at 08:17:37PM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > When using simple quotas we are not supposed to allocate qgroup records > when adding delayed references. However we allocate them if either mode > of quotas is enabled (the new simple one or the old one), but then we > never free them because running the accounting, which frees the records, > is only run when using the old quotas (at btrfs_qgroup_account_extents()), > resulting in a memory leak of the records allocated when adding delayed > references. > > Fix this by allocating the records only if the old quotas mode is enabled. > Also fix btrfs_qgroup_trace_extent_nolock() to return 1 if the old quotas > mode is not enabled - meaning the caller has to free the record. > > Fixes: 182940f4f4db ("btrfs: qgroup: add new quota mode for simple quotas") > Reported-by: syzbot+d3ddc6dcc6386dea398b@syzkaller.appspotmail.com > Link: https://lore.kernel.org/linux-btrfs/00000000000004769106097f9a34@google.com/ > Signed-off-by: Filipe Manana <fdmanana@suse.com> Added to misc-next, thanks.
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c index 9223934d95f4..891ea2fa263c 100644 --- a/fs/btrfs/delayed-ref.c +++ b/fs/btrfs/delayed-ref.c @@ -1041,7 +1041,7 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans, return -ENOMEM; } - if (btrfs_qgroup_enabled(fs_info) && !generic_ref->skip_qgroup) { + if (btrfs_qgroup_full_accounting(fs_info) && !generic_ref->skip_qgroup) { record = kzalloc(sizeof(*record), GFP_NOFS); if (!record) { kmem_cache_free(btrfs_delayed_tree_ref_cachep, ref); @@ -1144,7 +1144,7 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans, return -ENOMEM; } - if (btrfs_qgroup_enabled(fs_info) && !generic_ref->skip_qgroup) { + if (btrfs_qgroup_full_accounting(fs_info) && !generic_ref->skip_qgroup) { record = kzalloc(sizeof(*record), GFP_NOFS); if (!record) { kmem_cache_free(btrfs_delayed_data_ref_cachep, ref); diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index e48eba7e9379..ce446d9d7f23 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1888,7 +1888,7 @@ int btrfs_qgroup_trace_extent_nolock(struct btrfs_fs_info *fs_info, u64 bytenr = record->bytenr; if (!btrfs_qgroup_full_accounting(fs_info)) - return 0; + return 1; lockdep_assert_held(&delayed_refs->lock); trace_btrfs_qgroup_trace_extent(fs_info, record);