| Message ID | 20180830024536.4255-1-honli@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Hal Rosenstock |
| Headers | show |
| Series | [1/2] osm_prtn_config.c: Sanity check the value of mgroup_flag flag | expand |
On Thu, Aug 30, 2018 at 10:45:35AM +0800, Honggang LI wrote: > From: Honggang Li <honli@redhat.com> > > As all flags are unsigned integer in different size, sanity check the > value before covert the value to unsigned integer with 'strtoul'. > > Signed-off-by: Honggang Li <honli@redhat.com> > --- > opensm/osm_prtn_config.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/opensm/osm_prtn_config.c b/opensm/osm_prtn_config.c > index f76ad4d4..be81e794 100644 > --- a/opensm/osm_prtn_config.c > +++ b/opensm/osm_prtn_config.c > @@ -273,6 +273,20 @@ static int parse_group_flag(unsigned lineno, osm_log_t * p_log, > { > int rc = 0; > int len = strlen(flag); > + > + char *tmp = val; > + while (NULL != tmp && '\0' != *tmp) { Better to ask: while (tmp && .. > + if (!isxdigit(*tmp)) { > + OSM_LOG(p_log, OSM_LOG_VERBOSE, > + "PARSE WARN: line %d: " > + "suspicious val=(%s) detected. " > + "flag=(%s)\n", lineno, val, flag); > + return rc; > + } > + tmp++; > + } > + tmp = NULL; Since obviously tmp was defined in this patch i don't see a reason to set it to NULL here. > + > if (!strncmp(flag, "mtu", len)) { > rc = 1; > if (!val || (flags->mtu = strtoul(val, NULL, 0)) == 0) > -- > 2.14.4 >
On Thu, Aug 30, 2018 at 09:25:47PM +0300, Yuval Shaia wrote: > On Thu, Aug 30, 2018 at 10:45:35AM +0800, Honggang LI wrote: > > From: Honggang Li <honli@redhat.com> > > > > As all flags are unsigned integer in different size, sanity check the > > value before covert the value to unsigned integer with 'strtoul'. > > > > Signed-off-by: Honggang Li <honli@redhat.com> > > --- > > opensm/osm_prtn_config.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/opensm/osm_prtn_config.c b/opensm/osm_prtn_config.c > > index f76ad4d4..be81e794 100644 > > --- a/opensm/osm_prtn_config.c > > +++ b/opensm/osm_prtn_config.c > > @@ -273,6 +273,20 @@ static int parse_group_flag(unsigned lineno, osm_log_t * p_log, > > { > > int rc = 0; > > int len = strlen(flag); > > + > > + char *tmp = val; > > + while (NULL != tmp && '\0' != *tmp) { > > Better to ask: > while (tmp && .. fixed. > > > + if (!isxdigit(*tmp)) { > > + OSM_LOG(p_log, OSM_LOG_VERBOSE, > > + "PARSE WARN: line %d: " > > + "suspicious val=(%s) detected. " > > + "flag=(%s)\n", lineno, val, flag); > > + return rc; > > + } > > + tmp++; > > + } > > + tmp = NULL; > > Since obviously tmp was defined in this patch i don't see a reason to set > it to NULL here. ok, fixed it. please review v2 patch. > > > + > > if (!strncmp(flag, "mtu", len)) { > > rc = 1; > > if (!val || (flags->mtu = strtoul(val, NULL, 0)) == 0) > > -- > > 2.14.4 > >
diff --git a/opensm/osm_prtn_config.c b/opensm/osm_prtn_config.c index f76ad4d4..be81e794 100644 --- a/opensm/osm_prtn_config.c +++ b/opensm/osm_prtn_config.c @@ -273,6 +273,20 @@ static int parse_group_flag(unsigned lineno, osm_log_t * p_log, { int rc = 0; int len = strlen(flag); + + char *tmp = val; + while (NULL != tmp && '\0' != *tmp) { + if (!isxdigit(*tmp)) { + OSM_LOG(p_log, OSM_LOG_VERBOSE, + "PARSE WARN: line %d: " + "suspicious val=(%s) detected. " + "flag=(%s)\n", lineno, val, flag); + return rc; + } + tmp++; + } + tmp = NULL; + if (!strncmp(flag, "mtu", len)) { rc = 1; if (!val || (flags->mtu = strtoul(val, NULL, 0)) == 0)