On 23.05.2018 11:06, Su Yue wrote: > Commit 5a5003df98d5 ("btrfs: delayed-ref: double free in > btrfs_add_delayed_tree_ref()") fixed double free problem by creating > an unnessesary label to jump. > The elegant way is just to change "ref" to "head_ref" and keep > btrfs_add_delayed_tree_ref() and btrfs_add_delayed_data_ref() in > similar structure.
I agree, personally I'm a fan of multiple returns rather than jump labels, because at this point you know the function terminates and that's it. > > This patch reverts commit 5a5003df98d5 ("btrfs: delayed-ref: double > free in btrfs_add_delayed_tree_ref()") and frees the right head_ref. > No functional change. > > Signed-off-by: Su Yue <suy.f...@cn.fujitsu.com> Reviewed-by: Nikolay Borisov <nbori...@suse.com> > --- > This patch is based on for-next to avoid conflicts with patches > already in for-next. > > fs/btrfs/delayed-ref.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c > index 03dec673d12a..38f8d5d549ed 100644 > --- a/fs/btrfs/delayed-ref.c > +++ b/fs/btrfs/delayed-ref.c > @@ -741,14 +741,20 @@ int btrfs_add_delayed_tree_ref(struct btrfs_fs_info > *fs_info, > ref->level = level; > > head_ref = kmem_cache_alloc(btrfs_delayed_ref_head_cachep, GFP_NOFS); > - if (!head_ref) > - goto free_ref; > + if (!head_ref) { > + kmem_cache_free(btrfs_delayed_tree_ref_cachep, ref); > + return -ENOMEM; > + } > > if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags) && > is_fstree(ref_root)) { > record = kmalloc(sizeof(*record), GFP_NOFS); > - if (!record) > - goto free_head_ref; > + if (!record) { > + kmem_cache_free(btrfs_delayed_tree_ref_cachep, ref); > + kmem_cache_free(btrfs_delayed_ref_head_cachep, > + head_ref); > + return -ENOMEM; > + } > } > > init_delayed_ref_head(head_ref, record, bytenr, num_bytes, > @@ -779,13 +785,6 @@ int btrfs_add_delayed_tree_ref(struct btrfs_fs_info > *fs_info, > btrfs_qgroup_trace_extent_post(fs_info, record); > > return 0; > - > -free_head_ref: > - kmem_cache_free(btrfs_delayed_ref_head_cachep, head_ref); > -free_ref: > - kmem_cache_free(btrfs_delayed_tree_ref_cachep, ref); > - > - return -ENOMEM; > } > > /* > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html