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

[heming.zhao at suse.com]

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.

Thanks,
Heming
> 
>>   	return status;
>>   }
>>   
>