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

[Sheng Yong <shengyong1@huawei.com>]

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.
>> +
>> +		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;
>>
> 
> .
> 


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