From 47d41ad62c5d6008e72d2e9c137cf8f49ca86353 Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Mon, 17 Feb 2020 09:13:26 -0500 Subject: misc: fix coverity warnings Add an ALWAYS_ASSERT (...) macro, to (a) shut up coverity, and (b) check the indicated condition in production images. As in: p = hash_get(...); ALWAYS_ASSERT(p) /* was ASSERT(p) */ elt = pool_elt_at_index(pool, p[0]); This may not be the best way to handle a specific case, but failure to check return values at all followed by e.g. a pointer dereference isn't ok. Type: fix Ticket: VPP-1837 Signed-off-by: Dave Barach Change-Id: Ia97c641cefcfb7ea7d77ea5a55ed4afea0345acb --- src/plugins/gtpu/gtpu.c | 2 +- src/vnet/dpo/load_balance_map.c | 4 ++-- src/vnet/fib/fib_attached_export.c | 8 ++++---- src/vnet/geneve/encap.c | 7 +++++-- src/vnet/geneve/geneve.c | 2 +- src/vnet/ipsec/ipsec_output.c | 2 +- src/vnet/session/application_worker.c | 2 +- src/vnet/vxlan-gbp/vxlan_gbp.c | 2 +- src/vnet/vxlan-gpe/vxlan_gpe.c | 2 +- src/vnet/vxlan/vxlan.c | 4 ++-- src/vppinfra/error_bootstrap.h | 29 +++++++++++++++++++++++++++++ 11 files changed, 48 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/plugins/gtpu/gtpu.c b/src/plugins/gtpu/gtpu.c index 905ca5e1ca5..b0d7839f519 100644 --- a/src/plugins/gtpu/gtpu.c +++ b/src/plugins/gtpu/gtpu.c @@ -318,7 +318,7 @@ vtep_addr_unref (ip46_address_t * ip) uword *vtep = ip46_address_is_ip4 (ip) ? hash_get (gtpu_main.vtep4, ip->ip4.as_u32) : hash_get_mem (gtpu_main.vtep6, &ip->ip6); - ASSERT (vtep); + ALWAYS_ASSERT (vtep); if (--(*vtep) != 0) return *vtep; ip46_address_is_ip4 (ip) ? diff --git a/src/vnet/dpo/load_balance_map.c b/src/vnet/dpo/load_balance_map.c index bf5d1fb2943..7da360b88ac 100644 --- a/src/vnet/dpo/load_balance_map.c +++ b/src/vnet/dpo/load_balance_map.c @@ -274,7 +274,7 @@ load_balance_map_db_remove (load_balance_map_t *lbm) { p = hash_get(lb_maps_by_path_index, lbmp->lbmp_index); - ASSERT(NULL != p); + ALWAYS_ASSERT(NULL != p); fib_node_list_remove(p[0], lbmp->lbmp_sibling); } @@ -312,7 +312,7 @@ load_balance_map_fill (load_balance_map_t *lbm) tmp_buckets[jj++] = bucket++; } } - else + else { bucket += lbmp->lbmp_weight; } diff --git a/src/vnet/fib/fib_attached_export.c b/src/vnet/fib/fib_attached_export.c index 3c9c2bffee6..4a33b8897f5 100644 --- a/src/vnet/fib/fib_attached_export.c +++ b/src/vnet/fib/fib_attached_export.c @@ -364,7 +364,7 @@ fib_attached_export_purge (fib_entry_t *fib_entry) fed = fib_entry_delegate_find(export_entry, FIB_ENTRY_DELEGATE_ATTACHED_EXPORT); - ASSERT(NULL != fed); + ALWAYS_ASSERT(NULL != fed); export = pool_elt_at_index(fib_ae_export_pool, fed->fd_index); @@ -391,7 +391,7 @@ fib_attached_export_purge (fib_entry_t *fib_entry) pool_put(fib_ae_import_pool, import); fib_entry_delegate_remove(fib_entry, FIB_ENTRY_DELEGATE_ATTACHED_IMPORT); - } + } } void @@ -522,7 +522,7 @@ fib_ae_import_format (fib_node_index_t impi, s = format(s, "export-sibling:%d ", import->faei_export_sibling); s = format(s, "exporter:%d ", import->faei_exporter); s = format(s, "export-fib:%d ", import->faei_export_fib); - + s = format(s, "import-entry:%d ", import->faei_import_entry); s = format(s, "import-fib:%d ", import->faei_import_fib); @@ -544,7 +544,7 @@ fib_ae_export_format (fib_node_index_t expi, fib_ae_export_t *export; export = pool_elt_at_index(fib_ae_export_pool, expi); - + s = format(s, "\n Attached-Export:%d:[", (export - fib_ae_export_pool)); s = format(s, "export-entry:%d ", export->faee_ei); diff --git a/src/vnet/geneve/encap.c b/src/vnet/geneve/encap.c index af52fd24543..2859a9ae652 100644 --- a/src/vnet/geneve/encap.c +++ b/src/vnet/geneve/encap.c @@ -131,7 +131,7 @@ geneve_encap_inline (vlib_main_t * vm, next0 = t0->next_dpo.dpoi_next_node; } - ASSERT (t0 != NULL); + ALWAYS_ASSERT (t0 != NULL); vnet_buffer (b[0])->ip.adj_index[VLIB_TX] = t0->next_dpo.dpoi_index; @@ -145,7 +145,7 @@ geneve_encap_inline (vlib_main_t * vm, next1 = t1->next_dpo.dpoi_next_node; } - ASSERT (t1 != NULL); + ALWAYS_ASSERT (t1 != NULL); vnet_buffer (b[1])->ip.adj_index[VLIB_TX] = t1->next_dpo.dpoi_index; @@ -378,6 +378,9 @@ geneve_encap_inline (vlib_main_t * vm, /* Note: change to always set next0 if it may be set to drop */ next0 = t0->next_dpo.dpoi_next_node; } + + ALWAYS_ASSERT (t0 != NULL); + vnet_buffer (b[0])->ip.adj_index[VLIB_TX] = t0->next_dpo.dpoi_index; /* Apply the rewrite string. $$$$ vnet_rewrite? */ diff --git a/src/vnet/geneve/geneve.c b/src/vnet/geneve/geneve.c index 0c551a0545e..a54a5383c46 100644 --- a/src/vnet/geneve/geneve.c +++ b/src/vnet/geneve/geneve.c @@ -312,7 +312,7 @@ vtep_addr_unref (ip46_address_t * ip) uword *vtep = ip46_address_is_ip4 (ip) ? hash_get (geneve_main.vtep4, ip->ip4.as_u32) : hash_get_mem (geneve_main.vtep6, &ip->ip6); - ASSERT (vtep); + ALWAYS_ASSERT (vtep); if (--(*vtep) != 0) return *vtep; ip46_address_is_ip4 (ip) ? diff --git a/src/vnet/ipsec/ipsec_output.c b/src/vnet/ipsec/ipsec_output.c index 2ba965a3ae8..d09a027de29 100644 --- a/src/vnet/ipsec/ipsec_output.c +++ b/src/vnet/ipsec/ipsec_output.c @@ -222,7 +222,7 @@ ipsec_output_inline (vlib_main_t * vm, vlib_node_runtime_t * node, if (PREDICT_FALSE (last_sw_if_index != sw_if_index0)) { uword *p = hash_get (im->spd_index_by_sw_if_index, sw_if_index0); - ASSERT (p); + ALWAYS_ASSERT (p); spd_index0 = p[0]; spd0 = pool_elt_at_index (im->spds, spd_index0); last_sw_if_index = sw_if_index0; diff --git a/src/vnet/session/application_worker.c b/src/vnet/session/application_worker.c index 1db6baab74e..0b67d2922bf 100644 --- a/src/vnet/session/application_worker.c +++ b/src/vnet/session/application_worker.c @@ -468,7 +468,7 @@ app_worker_get_listen_segment_manager (app_worker_t * app, { uword *smp; smp = hash_get (app->listeners_table, listen_session_get_handle (listener)); - ASSERT (smp != 0); + ALWAYS_ASSERT (smp != 0); return segment_manager_get (*smp); } diff --git a/src/vnet/vxlan-gbp/vxlan_gbp.c b/src/vnet/vxlan-gbp/vxlan_gbp.c index 80eecb46c10..9a8158e9ab0 100644 --- a/src/vnet/vxlan-gbp/vxlan_gbp.c +++ b/src/vnet/vxlan-gbp/vxlan_gbp.c @@ -291,7 +291,7 @@ vtep_addr_unref (ip46_address_t * ip) uword *vtep = ip46_address_is_ip4 (ip) ? hash_get (vxlan_gbp_main.vtep4, ip->ip4.as_u32) : hash_get_mem (vxlan_gbp_main.vtep6, &ip->ip6); - ASSERT (vtep); + ALWAYS_ASSERT (vtep); if (--(*vtep) != 0) return *vtep; ip46_address_is_ip4 (ip) ? diff --git a/src/vnet/vxlan-gpe/vxlan_gpe.c b/src/vnet/vxlan-gpe/vxlan_gpe.c index 86157c0b519..e5ff4f74295 100644 --- a/src/vnet/vxlan-gpe/vxlan_gpe.c +++ b/src/vnet/vxlan-gpe/vxlan_gpe.c @@ -404,7 +404,7 @@ vtep_addr_unref (ip46_address_t * ip) uword *vtep = ip46_address_is_ip4 (ip) ? hash_get (vxlan_gpe_main.vtep4, ip->ip4.as_u32) : hash_get_mem (vxlan_gpe_main.vtep6, &ip->ip6); - ASSERT (vtep); + ALWAYS_ASSERT (vtep); if (--(*vtep) != 0) return *vtep; ip46_address_is_ip4 (ip) ? diff --git a/src/vnet/vxlan/vxlan.c b/src/vnet/vxlan/vxlan.c index 2b852511325..32647496a76 100644 --- a/src/vnet/vxlan/vxlan.c +++ b/src/vnet/vxlan/vxlan.c @@ -311,7 +311,7 @@ vtep_addr_unref (ip46_address_t * ip) uword *vtep = ip46_address_is_ip4 (ip) ? hash_get (vxlan_main.vtep4, ip->ip4.as_u32) : hash_get_mem (vxlan_main.vtep6, &ip->ip6); - ASSERT (vtep); + ALWAYS_ASSERT (vtep); if (--(*vtep) != 0) return *vtep; ip46_address_is_ip4 (ip) ? @@ -337,7 +337,7 @@ mcast_shared_get (ip46_address_t * ip) { ASSERT (ip46_address_is_multicast (ip)); uword *p = hash_get_mem (vxlan_main.mcast_shared, ip); - ASSERT (p); + ALWAYS_ASSERT (p); mcast_shared_t ret = {.as_u64 = *p }; return ret; } diff --git a/src/vppinfra/error_bootstrap.h b/src/vppinfra/error_bootstrap.h index 1cd91058574..8dbbc7f359d 100644 --- a/src/vppinfra/error_bootstrap.h +++ b/src/vppinfra/error_bootstrap.h @@ -79,6 +79,35 @@ do { \ } \ } while (0) +/* + * This version always generates code, and has a Coverity-specific + * version to stop Coverity complaining about + * ALWAYS_ASSERT(p != 0); p->member... + */ + +#ifndef __COVERITY__ +#define ALWAYS_ASSERT(truth) \ +do { \ + if (PREDICT_FALSE(!(truth))) \ + { \ + _clib_error (CLIB_ERROR_ABORT, 0, 0, \ + "%s:%d (%s) assertion `%s' fails", \ + __FILE__, \ + (uword) __LINE__, \ + clib_error_function, \ + # truth); \ + } \ +} while (0) +#else /* __COVERITY__ */ +#define ALWAYS_ASSERT(truth) \ +do { \ + if (PREDICT_FALSE(!(truth))) \ + { \ + abort(); \ + } \ +} while (0) +#endif /* __COVERITY */ + #if defined(__clang__) #define STATIC_ASSERT(truth,...) #else -- cgit 1.2.3-korg