| Message ID | 20170130143530.14944-1-bill.fischofer@linaro.org |
|---|---|
| State | Accepted |
| Commit | dd494a2b6a22f0516ce8890df05de826377fdae2 |
| Headers | show |
Hi Bill Did you run this in GitHub coverity_scan and did it clear the issue for you ? I ran it, but I did not have a before view for that nice fuzzy feeling of seeing it dissapear, only that I did not get informed of anything new with your patch applied. I think my results should be public if you have a github login, and the change looks correct. https://travis-ci.org/mike-holmes-linaro/odp/builds/196630581 https://scan.coverity.com/projects/mike-holmes-linaro-odp/view_defects Mike On 30 January 2017 at 09:35, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2862 by checking > pointer validity before dereferencing. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > Reviewed-by: Mike Holmes <mike.holmes@linaro.org> > --- > helper/iplookuptable.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/helper/iplookuptable.c b/helper/iplookuptable.c > index aaebea3..845125b 100644 > --- a/helper/iplookuptable.c > +++ b/helper/iplookuptable.c > @@ -666,12 +666,14 @@ odph_iplookup_table_put_value(odph_table_t tbl, > void *key, void *value) > odph_iplookup_table_impl *impl = (void *)tbl; > odph_iplookup_prefix_t *prefix = (odph_iplookup_prefix_t *)key; > prefix_entry_t *l1e = NULL; > - odp_buffer_t nexthop = *((odp_buffer_t *)value); > + odp_buffer_t nexthop; > int ret = 0; > > if ((tbl == NULL) || (key == NULL) || (value == NULL)) > return -1; > > + nexthop = *((odp_buffer_t *)value); > + > if (prefix->cidr == 0) > return -1; > prefix->ip = prefix->ip & (0xffffffff << (IP_LENGTH - > prefix->cidr)); > @@ -708,13 +710,16 @@ int odph_iplookup_table_get_value(odph_table_t tbl, > void *key, > uint32_t buffer_size ODP_UNUSED) > { > odph_iplookup_table_impl *impl = (void *)tbl; > - uint32_t ip = *((uint32_t *)key); > - prefix_entry_t *entry = &impl->l1e[ip >> 16]; > + uint32_t ip; > + prefix_entry_t *entry; > odp_buffer_t *buff = (odp_buffer_t *)buffer; > > if ((tbl == NULL) || (key == NULL) || (buffer == NULL)) > return -EINVAL; > > + ip = *((uint32_t *)key); > + entry = &impl->l1e[ip >> 16]; > + > if (entry == NULL) { > ODPH_DBG("failed to get L1 entry.\n"); > return -1; > @@ -881,13 +886,16 @@ odph_iplookup_table_remove_value(odph_table_t tbl, > void *key) > { > odph_iplookup_table_impl *impl = (void *)tbl; > odph_iplookup_prefix_t *prefix = (odph_iplookup_prefix_t *)key; > - uint32_t ip = prefix->ip; > - uint8_t cidr = prefix->cidr; > + uint32_t ip; > + uint8_t cidr; > > if ((tbl == NULL) || (key == NULL)) > return -EINVAL; > > - if (!prefix->cidr) > + ip = prefix->ip; > + cidr = prefix->cidr; > + > + if (cidr == 0) > return -EINVAL; > > prefix_entry_t *entry = &impl->l1e[ip >> 16]; > -- > 2.9.3 > > -- Mike Holmes Program Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
On 01/30/17 19:56, Mike Holmes wrote: > Hi Bill > > Did you run this in GitHub coverity_scan and did it clear the issue for you > ? > > I ran it, but I did not have a before view for that nice fuzzy feeling of > seeing it dissapear, only that I did not get informed of anything new with > your patch applied. > I think my results should be public if you have a github login, and the > change looks correct. > > https://travis-ci.org/mike-holmes-linaro/odp/builds/196630581 > https://scan.coverity.com/projects/mike-holmes-linaro-odp/view_defects > > Mike > > On 30 January 2017 at 09:35, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2862 by checking >> pointer validity before dereferencing. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> > > Reviewed-by: Mike Holmes <mike.holmes@linaro.org> > Merged, Maxim. > > >> --- >> helper/iplookuptable.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/helper/iplookuptable.c b/helper/iplookuptable.c >> index aaebea3..845125b 100644 >> --- a/helper/iplookuptable.c >> +++ b/helper/iplookuptable.c >> @@ -666,12 +666,14 @@ odph_iplookup_table_put_value(odph_table_t tbl, >> void *key, void *value) >> odph_iplookup_table_impl *impl = (void *)tbl; >> odph_iplookup_prefix_t *prefix = (odph_iplookup_prefix_t *)key; >> prefix_entry_t *l1e = NULL; >> - odp_buffer_t nexthop = *((odp_buffer_t *)value); >> + odp_buffer_t nexthop; >> int ret = 0; >> >> if ((tbl == NULL) || (key == NULL) || (value == NULL)) >> return -1; >> >> + nexthop = *((odp_buffer_t *)value); >> + >> if (prefix->cidr == 0) >> return -1; >> prefix->ip = prefix->ip & (0xffffffff << (IP_LENGTH - >> prefix->cidr)); >> @@ -708,13 +710,16 @@ int odph_iplookup_table_get_value(odph_table_t tbl, >> void *key, >> uint32_t buffer_size ODP_UNUSED) >> { >> odph_iplookup_table_impl *impl = (void *)tbl; >> - uint32_t ip = *((uint32_t *)key); >> - prefix_entry_t *entry = &impl->l1e[ip >> 16]; >> + uint32_t ip; >> + prefix_entry_t *entry; >> odp_buffer_t *buff = (odp_buffer_t *)buffer; >> >> if ((tbl == NULL) || (key == NULL) || (buffer == NULL)) >> return -EINVAL; >> >> + ip = *((uint32_t *)key); >> + entry = &impl->l1e[ip >> 16]; >> + >> if (entry == NULL) { >> ODPH_DBG("failed to get L1 entry.\n"); >> return -1; >> @@ -881,13 +886,16 @@ odph_iplookup_table_remove_value(odph_table_t tbl, >> void *key) >> { >> odph_iplookup_table_impl *impl = (void *)tbl; >> odph_iplookup_prefix_t *prefix = (odph_iplookup_prefix_t *)key; >> - uint32_t ip = prefix->ip; >> - uint8_t cidr = prefix->cidr; >> + uint32_t ip; >> + uint8_t cidr; >> >> if ((tbl == NULL) || (key == NULL)) >> return -EINVAL; >> >> - if (!prefix->cidr) >> + ip = prefix->ip; >> + cidr = prefix->cidr; >> + >> + if (cidr == 0) >> return -EINVAL; >> >> prefix_entry_t *entry = &impl->l1e[ip >> 16]; >> -- >> 2.9.3 >> >> > >
diff --git a/helper/iplookuptable.c b/helper/iplookuptable.c index aaebea3..845125b 100644 --- a/helper/iplookuptable.c +++ b/helper/iplookuptable.c @@ -666,12 +666,14 @@ odph_iplookup_table_put_value(odph_table_t tbl, void *key, void *value) odph_iplookup_table_impl *impl = (void *)tbl; odph_iplookup_prefix_t *prefix = (odph_iplookup_prefix_t *)key; prefix_entry_t *l1e = NULL; - odp_buffer_t nexthop = *((odp_buffer_t *)value); + odp_buffer_t nexthop; int ret = 0; if ((tbl == NULL) || (key == NULL) || (value == NULL)) return -1; + nexthop = *((odp_buffer_t *)value); + if (prefix->cidr == 0) return -1; prefix->ip = prefix->ip & (0xffffffff << (IP_LENGTH - prefix->cidr)); @@ -708,13 +710,16 @@ int odph_iplookup_table_get_value(odph_table_t tbl, void *key, uint32_t buffer_size ODP_UNUSED) { odph_iplookup_table_impl *impl = (void *)tbl; - uint32_t ip = *((uint32_t *)key); - prefix_entry_t *entry = &impl->l1e[ip >> 16]; + uint32_t ip; + prefix_entry_t *entry; odp_buffer_t *buff = (odp_buffer_t *)buffer; if ((tbl == NULL) || (key == NULL) || (buffer == NULL)) return -EINVAL; + ip = *((uint32_t *)key); + entry = &impl->l1e[ip >> 16]; + if (entry == NULL) { ODPH_DBG("failed to get L1 entry.\n"); return -1; @@ -881,13 +886,16 @@ odph_iplookup_table_remove_value(odph_table_t tbl, void *key) { odph_iplookup_table_impl *impl = (void *)tbl; odph_iplookup_prefix_t *prefix = (odph_iplookup_prefix_t *)key; - uint32_t ip = prefix->ip; - uint8_t cidr = prefix->cidr; + uint32_t ip; + uint8_t cidr; if ((tbl == NULL) || (key == NULL)) return -EINVAL; - if (!prefix->cidr) + ip = prefix->ip; + cidr = prefix->cidr; + + if (cidr == 0) return -EINVAL; prefix_entry_t *entry = &impl->l1e[ip >> 16];
Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2862 by checking pointer validity before dereferencing. Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- helper/iplookuptable.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) -- 2.9.3