linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* Re: [RFC PATCH] mkfs.f2fs: avoid dumplicate extension
  2015-11-27 16:55 [RFC PATCH] mkfs.f2fs: avoid dumplicate extension Sheng Yong
@ 2015-11-27 13:09 ` Chao Yu
  2015-11-28  0:39   ` Sheng Yong
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2015-11-27 13:09 UTC (permalink / raw)
  To: Sheng Yong; +Cc: jaegeuk, linux-f2fs-devel

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.

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

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

Thanks,

> +			memcpy(sb.extension_list[i++], ue, name_len);
>  		ue = strtok(NULL, ",");
>  		if (i >= F2FS_MAX_EXTENSION)
>  			break;
> 

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC PATCH] mkfs.f2fs: avoid dumplicate extension
@ 2015-11-27 16:55 Sheng Yong
  2015-11-27 13:09 ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Sheng Yong @ 2015-11-27 16:55 UTC (permalink / raw)
  To: chao2.yu, jaegeuk; +Cc: linux-f2fs-devel

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)
+{
+	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);
+
+		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);
-		memcpy(sb.extension_list[i++], ue, name_len);
+		if (!extension_exist(ue, name_len))
+			memcpy(sb.extension_list[i++], ue, name_len);
 		ue = strtok(NULL, ",");
 		if (i >= F2FS_MAX_EXTENSION)
 			break;
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] mkfs.f2fs: avoid dumplicate extension
  2015-11-27 13:09 ` Chao Yu
@ 2015-11-28  0:39   ` Sheng Yong
  2015-11-28  2:31     ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Sheng Yong @ 2015-11-28  0:39 UTC (permalink / raw)
  To: Chao Yu; +Cc: jaegeuk, linux-f2fs-devel


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


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] mkfs.f2fs: avoid dumplicate extension
  2015-11-28  0:39   ` Sheng Yong
@ 2015-11-28  2:31     ` Chao Yu
  2015-11-28  2:45       ` Sheng Yong
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2015-11-28  2:31 UTC (permalink / raw)
  To: Sheng Yong; +Cc: jaegeuk, linux-f2fs-devel

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?

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

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] mkfs.f2fs: avoid dumplicate extension
  2015-11-28  2:31     ` Chao Yu
@ 2015-11-28  2:45       ` Sheng Yong
  0 siblings, 0 replies; 5+ messages in thread
From: Sheng Yong @ 2015-11-28  2:45 UTC (permalink / raw)
  To: Chao Yu; +Cc: jaegeuk, linux-f2fs-devel



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


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-11-28  2:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-27 16:55 [RFC PATCH] mkfs.f2fs: avoid dumplicate extension Sheng Yong
2015-11-27 13:09 ` Chao Yu
2015-11-28  0:39   ` Sheng Yong
2015-11-28  2:31     ` Chao Yu
2015-11-28  2:45       ` Sheng Yong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).