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

[heming.zhao at suse.com]

On 4/11/22 09:56, Joseph Qi wrote:
> 
> 
> On 4/10/22 12:47 AM, heming.zhao at 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 at 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.
> 

OK, I will make a complete patch on next verion.
And I found Thunderbird automatically adds '\n' in reply mail, which make the mail
format messy. I am sorry for this, and try to switch from Thunderbird to mutt.

Thanks,
Heming