Re: [Ocfs2-devel] [PATCH v1 3/5] ocfs2: ocfs2_initialize_super does cleanup job before return error - Joseph Qi via Ocfs2-devel

From: Joseph Qi via Ocfs2-devel <ocfs2-devel@oss.oracle.com>
To: "heming.zhao@suse.com" <heming.zhao@suse.com>,
	ocfs2-devel@oss.oracle.com
Subject: Re: [Ocfs2-devel] [PATCH v1 3/5] ocfs2: ocfs2_initialize_super does cleanup job before return error
Date: Mon, 11 Apr 2022 09:56:09 +0800	[thread overview]
Message-ID: <93529adf-73bf-5818-dbac-a9276512f177@linux.alibaba.com> (raw)
In-Reply-To: <4248f4f2-6212-247d-6d14-05a02d7bc249@suse.com>

On 4/10/22 12:47 AM, heming.zhao@suse.com wrote:
> On 4/9/22 21:30, Joseph Qi wrote:
>>
>>
>> On 4/8/22 6:30 PM, Heming Zhao wrote:
>>> After this patch, when error, ocfs2_fill_super doesn't take care to
>>> release resources which are allocated in ocfs2_initialize_super.
>>>
>>> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
>>> ---
>>>   fs/ocfs2/super.c | 58 +++++++++++++++++++++++++++++++++---------------
>>>   1 file changed, 40 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
>>> index f91c5510bc7e..8443ba031dec 100644
>>> --- a/fs/ocfs2/super.c
>>> +++ b/fs/ocfs2/super.c
>>> @@ -2023,7 +2023,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
>>>       if (!osb) {
>>>           status = -ENOMEM;
>>>           mlog_errno(status);
>>> -        goto bail;
>>> +        goto out;
>>>       }
>>> ... ...
>>>   -bail:
>>> +    return status;
>>> +
>>> +out_slot_info:
>>> +    ocfs2_free_slot_info(osb);
>>> +out_system_inodes:
>>> +    ocfs2_release_system_inodes(osb);
>>> +out_dlm_out:
>>> +    ocfs2_put_dlm_debug(osb->osb_dlm_debug);
>>> +out_uuid_str:
>>> +    kfree(osb->uuid_str);
>>> +out_journal:
>>> +    kfree(osb->journal);
>>> +out_orphan_wipes:
>>> +    kfree(osb->osb_orphan_wipes);
>>> +out_slot_recovery_gen:
>>> +    kfree(osb->slot_recovery_generations);
>>> +out_vol_label:
>>> +    kfree(osb->vol_label);
>>> +out_recovery_map:
>>> +    kfree(osb->recovery_map);
>>> +out:
>>> +    kfree(osb);
>>
>> Should set osb to NULL here to prevent UAF in ocfs2_fill_super().
>>
> 
> Your concern only valid with patch 1/5+2/5+3/5, but after 5/5, the UAF won't be
> triggered. ocfs2_initialize_super() failure will directly jump (by "goto out")
> to return.
> 

Right, but we'd better make this patch look more complete.

Thanks,
Joseph

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel