Re: [RFC PATCH] mkfs.f2fs: avoid dumplicate extension

[Sheng Yong <shengyong1@huawei.com>]

On 11/28/2015 10:31 AM, Chao Yu wrote:
> Hi Sheng Yong,
> 
> On 11/28/15 8:39 AM, Sheng Yong wrote:
>>
>> On 11/27/2015 9:09 PM, Chao Yu wrote:
>>> Hi Sheng Yong,
>>>
>>> On 11/28/15 12:55 AM, Sheng Yong wrote:
>>>> Before copying user specified extension to extension_list, check if it is
>>>> already in the list.
>>>>
>>>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>>>> ---
>>>>  mkfs/f2fs_format.c | 23 ++++++++++++++++++++++-
>>>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>>>> index 176bdea..b7ea19f 100644
>>>> --- a/mkfs/f2fs_format.c
>>>> +++ b/mkfs/f2fs_format.c
>>>> @@ -118,6 +118,26 @@ const char *media_ext_lists[] = {
>>>>  	NULL
>>>>  };
>>>>  
>>>> +static int extension_exist(const char *name, const int len)
>>>
>>> bool is_extension_exist?
>>>
>>>> +{
>>>> +	const char **extlist = media_ext_lists;
>>>> +	int name_len, min_len;
>>>> +	int ret;
>>>> +
>>>> +
>>>> +	while (*extlist) {
>>>> +		name_len = strlen(*extlist);
>>>> +		min_len = name_len < len ? name_len : len;
>>>> +		ret = strncmp(*extlist, name, min_len);
>>>
>>> How about using strcmp(*extlist, name)? It will return 0 only if the two strings
>>> are the same.
>>>
>> Hi, Chao Yu
>>
>> Thanks for your reply. I was considering strcmp may have security issue.
> 
> Could you share details about the security issue of using strcmp?
I was wrong, please ignore this :(

thanks,
Sheng
> 
> Thanks,
> 
>>>> +
>>>> +		if (ret == 0 && name_len == len)
>>>> +			return 1;
>>>> +		extlist++;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>>  static void configure_extension_list(void)
>>>>  {
>>>>  	const char **extlist = media_ext_lists;
>>>> @@ -144,7 +164,8 @@ static void configure_extension_list(void)
>>>>  	ue = strtok(ext_str, ",");
>>>>  	while (ue != NULL) {
>>>>  		name_len = strlen(ue);
>>>
>>> We should verify the length of extension to avoid memcpy overflow. I will send a
>>> patch for fixing.
>> I agree. And I think we should make sure there is a '\0' at the end of each string
>> in extension_list.
>>>
>>>> -		memcpy(sb.extension_list[i++], ue, name_len);
>>>> +		if (!extension_exist(ue, name_len))
>>>
>>> How do you think of comparing extension user defined with sb.extension_list?
>>> In this way, duplication user defined can be avoided completely.
>> Right. I'll send a v2 latter.
>>
>> thanks,
>> Sheng
>>>
>>> Thanks,
>>>
>>>> +			memcpy(sb.extension_list[i++], ue, name_len);
>>>>  		ue = strtok(NULL, ",");
>>>>  		if (i >= F2FS_MAX_EXTENSION)
>>>>  			break;
>>>>
>>>
>>> .
>>>
>>
> 
> .
> 


------------------------------------------------------------------------------