| Message ID | 20181122025156.42217-1-honnappa.nagarahalli@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | hash: fix out-of-bound write while freeing key slot | expand |
On Wed, Nov 21, 2018 at 08:51:56PM -0600, Honnappa Nagarahalli wrote: > Add a debug check for out-of-bound write while freeing the key slot. > > Coverity issue: 325733 > Fixes: e605a1d36ca7 ("hash: add lock-free r/w concurrency") > Cc: honnappa.nagarahalli@arm.com > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > Reviewed-by: Steve Capper <steve.capper@arm.com> > Reviewed-by: Gavin Hu <gavin.hu@arm.com> > --- > lib/librte_hash/rte_cuckoo_hash.c | 4 ++++ > lib/librte_hash/rte_cuckoo_hash.h | 11 +++++++++++ > 2 files changed, 15 insertions(+) > While I think enqueuing a free slot should ever fail, I don't see an issue with having a check for that. Acked-by: Bruce Richardson <bruce.richardson@intel.com>
20/12/2018 14:59, Bruce Richardson: > On Wed, Nov 21, 2018 at 08:51:56PM -0600, Honnappa Nagarahalli wrote: > > Add a debug check for out-of-bound write while freeing the key slot. > > > > Coverity issue: 325733 > > Fixes: e605a1d36ca7 ("hash: add lock-free r/w concurrency") > > Cc: honnappa.nagarahalli@arm.com > > > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> > > Reviewed-by: Steve Capper <steve.capper@arm.com> > > Reviewed-by: Gavin Hu <gavin.hu@arm.com> > > --- > > lib/librte_hash/rte_cuckoo_hash.c | 4 ++++ > > lib/librte_hash/rte_cuckoo_hash.h | 11 +++++++++++ > > 2 files changed, 15 insertions(+) > > > While I think enqueuing a free slot should ever fail, I don't see an issue > with having a check for that. > > Acked-by: Bruce Richardson <bruce.richardson@intel.com> Applied, thanks
diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 5ddcccd87..c35a60266 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -1272,6 +1272,9 @@ remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt, unsigned i) n_slots = rte_ring_mp_enqueue_burst(h->free_slots, cached_free_slots->objs, LCORE_CACHE_SIZE, NULL); + ERR_IF_TRUE((n_slots == 0), + "%s: could not enqueue free slots in global ring\n", + __func__); cached_free_slots->len -= n_slots; } /* Put index of new free slot in cache. */ @@ -1477,6 +1480,7 @@ rte_hash_free_key_with_position(const struct rte_hash *h, n_slots = rte_ring_mp_enqueue_burst(h->free_slots, cached_free_slots->objs, LCORE_CACHE_SIZE, NULL); + RETURN_IF_TRUE((n_slots == 0), -EFAULT); cached_free_slots->len -= n_slots; } /* Put index of new free slot in cache. */ diff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuckoo_hash.h index 5dfbbc48b..eacdaa8d4 100644 --- a/lib/librte_hash/rte_cuckoo_hash.h +++ b/lib/librte_hash/rte_cuckoo_hash.h @@ -29,6 +29,17 @@ #define RETURN_IF_TRUE(cond, retval) #endif +#if defined(RTE_LIBRTE_HASH_DEBUG) +#define ERR_IF_TRUE(cond, fmt, args...) do { \ + if (cond) { \ + RTE_LOG(ERR, HASH, fmt, ##args); \ + return; \ + } \ +} while (0) +#else +#define ERR_IF_TRUE(cond, fmt, args...) +#endif + #include <rte_hash_crc.h> #include <rte_jhash.h>
Add a debug check for out-of-bound write while freeing the key slot. Coverity issue: 325733 Fixes: e605a1d36ca7 ("hash: add lock-free r/w concurrency") Cc: honnappa.nagarahalli@arm.com Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Steve Capper <steve.capper@arm.com> Reviewed-by: Gavin Hu <gavin.hu@arm.com> --- lib/librte_hash/rte_cuckoo_hash.c | 4 ++++ lib/librte_hash/rte_cuckoo_hash.h | 11 +++++++++++ 2 files changed, 15 insertions(+) -- 2.17.1