* [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 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
* 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).