From c7eb30d5b54a58a52316d9b330a2dfad80ebdfae Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 5 Feb 2025 08:42:00 -0500 Subject: [PATCH 1/2] bfdd: Use pass by reference instead of pass by value for a struct The function bfd_key_lookup is currently sending by value for a now very large structure. Let's convert this over to pass by reference. This is noticed by coverity. Signed-off-by: Donald Sharp (cherry picked from commit 6d80d0c595fd073c56f4fc5b3cd5568ef8a9d5ae) --- bfdd/bfd.c | 8 ++++---- bfdd/bfd.h | 2 +- bfdd/bfdd_nb_config.c | 6 +++--- bfdd/bfdd_nb_state.c | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/bfdd/bfd.c b/bfdd/bfd.c index 164910556b..3a2959a03a 100644 --- a/bfdd/bfd.c +++ b/bfdd/bfd.c @@ -280,7 +280,7 @@ struct bfd_session *bs_peer_find(struct bfd_peer_cfg *bpc) gen_bfd_key(&key, &bpc->bpc_peer, &bpc->bpc_local, bpc->bpc_mhop, bpc->bpc_localif, bpc->bpc_vrfname, bpc->bfd_name); - return bfd_key_lookup(key); + return bfd_key_lookup(&key); } /* @@ -770,7 +770,7 @@ struct bfd_session *ptm_bfd_sess_find(struct bfd_pkt *cp, vrf ? vrf->name : VRF_DEFAULT_NAME, NULL); /* XXX maybe remoteDiscr should be checked for remoteHeard cases. */ - return bfd_key_lookup(key); + return bfd_key_lookup(&key); } void bfd_xmt_cb(struct event *t) @@ -1962,11 +1962,11 @@ struct bfd_session *bfd_id_lookup(uint32_t id) return hash_lookup(bfd_id_hash, &bs); } -struct bfd_session *bfd_key_lookup(struct bfd_key key) +struct bfd_session *bfd_key_lookup(struct bfd_key *key) { struct bfd_session bs; - bs.key = key; + bs.key = *key; return hash_lookup(bfd_key_hash, &bs); } diff --git a/bfdd/bfd.h b/bfdd/bfd.h index d9119d16c2..ea8ed03ff9 100644 --- a/bfdd/bfd.h +++ b/bfdd/bfd.h @@ -698,7 +698,7 @@ void bfd_vrf_init(void); void bfd_vrf_terminate(void); struct bfd_vrf_global *bfd_vrf_look_by_session(struct bfd_session *bfd); struct bfd_session *bfd_id_lookup(uint32_t id); -struct bfd_session *bfd_key_lookup(struct bfd_key key); +struct bfd_session *bfd_key_lookup(struct bfd_key *key); struct sbfd_reflector *sbfd_discr_lookup(uint32_t discr); struct bfd_session *bfd_id_delete(uint32_t id); struct bfd_session *bfd_key_delete(struct bfd_key key); diff --git a/bfdd/bfdd_nb_config.c b/bfdd/bfdd_nb_config.c index 15da1e2440..f553d56652 100644 --- a/bfdd/bfdd_nb_config.c +++ b/bfdd/bfdd_nb_config.c @@ -220,7 +220,7 @@ static int bfd_session_create(struct nb_cb_create_args *args, bool mhop, uint32_ case NB_EV_PREPARE: if (bfd_mode == BFD_MODE_TYPE_BFD) { bfd_session_get_key(mhop, args->dnode, &bk); - bs = bfd_key_lookup(bk); + bs = bfd_key_lookup(&bk); /* This session was already configured by another daemon. */ if (bs != NULL) { @@ -249,7 +249,7 @@ static int bfd_session_create(struct nb_cb_create_args *args, bool mhop, uint32_ } else if (bfd_mode == BFD_MODE_TYPE_SBFD_ECHO || bfd_mode == BFD_MODE_TYPE_SBFD_INIT) { sbfd_session_get_key(mhop, args->dnode, &bk); - bs = bfd_key_lookup(bk); + bs = bfd_key_lookup(&bk); /* This session was already configured by another daemon. */ if (bs != NULL) { @@ -369,7 +369,7 @@ static int bfd_session_destroy(enum nb_event event, const struct lyd_node *dnode else sbfd_session_get_key(mhop, dnode, &bk); - if (bfd_key_lookup(bk) == NULL) + if (bfd_key_lookup(&bk) == NULL) return NB_ERR_INCONSISTENCY; break; diff --git a/bfdd/bfdd_nb_state.c b/bfdd/bfdd_nb_state.c index c528478231..152d01e568 100644 --- a/bfdd/bfdd_nb_state.c +++ b/bfdd/bfdd_nb_state.c @@ -52,7 +52,7 @@ bfdd_bfd_sessions_single_hop_lookup_entry(struct nb_cb_lookup_entry_args *args) memset(&lsa, 0, sizeof(lsa)); gen_bfd_key(&bk, &psa, &lsa, false, ifname, vrf, NULL); - return bfd_key_lookup(bk); + return bfd_key_lookup(&bk); } /* @@ -356,7 +356,7 @@ bfdd_bfd_sessions_multi_hop_lookup_entry(struct nb_cb_lookup_entry_args *args) strtosa(source_addr, &lsa); gen_bfd_key(&bk, &psa, &lsa, true, NULL, vrf, NULL); - return bfd_key_lookup(bk); + return bfd_key_lookup(&bk); } /* @@ -394,7 +394,7 @@ const void *bfdd_bfd_sessions_sbfd_echo_lookup_entry(struct nb_cb_lookup_entry_a memset(&psa, 0, sizeof(psa)); gen_bfd_key(&bk, &psa, &lsa, true, NULL, vrf, bfdname); - return bfd_key_lookup(bk); + return bfd_key_lookup(&bk); } /* @@ -436,5 +436,5 @@ const void *bfdd_bfd_sessions_sbfd_init_lookup_entry(struct nb_cb_lookup_entry_a strtosa(dest_addr, &psa); gen_bfd_key(&bk, &psa, &lsa, true, NULL, vrf, bfdname); - return bfd_key_lookup(bk); + return bfd_key_lookup(&bk); } From 461f2c327d660f5c3d8950b3dc83945957bb3a78 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 5 Feb 2025 08:47:31 -0500 Subject: [PATCH 2/2] bfdd: Use pass by reference for bfd_key_delete Coverity is pointing out that bfd_key_delete is passing by value instead of reference for a very large structure. Double plus not good. Signed-off-by: Donald Sharp (cherry picked from commit 8119e167b0ae95927618696ba11d7252d9d5637c) --- bfdd/bfd.c | 6 +++--- bfdd/bfd.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bfdd/bfd.c b/bfdd/bfd.c index 3a2959a03a..e536567c36 100644 --- a/bfdd/bfd.c +++ b/bfdd/bfd.c @@ -982,7 +982,7 @@ void bfd_session_free(struct bfd_session *bs) /* Remove session from data plane if any. */ bfd_dplane_delete_session(bs); - bfd_key_delete(bs->key); + bfd_key_delete(&bs->key); bfd_id_delete(bs->discrs.my_discr); /* Remove observer if any. */ @@ -1999,11 +1999,11 @@ struct bfd_session *bfd_id_delete(uint32_t id) return hash_release(bfd_id_hash, &bs); } -struct bfd_session *bfd_key_delete(struct bfd_key key) +struct bfd_session *bfd_key_delete(struct bfd_key *key) { struct bfd_session bs; - bs.key = key; + bs.key = *key; return hash_release(bfd_key_hash, &bs); } diff --git a/bfdd/bfd.h b/bfdd/bfd.h index ea8ed03ff9..645a76596f 100644 --- a/bfdd/bfd.h +++ b/bfdd/bfd.h @@ -701,7 +701,7 @@ struct bfd_session *bfd_id_lookup(uint32_t id); struct bfd_session *bfd_key_lookup(struct bfd_key *key); struct sbfd_reflector *sbfd_discr_lookup(uint32_t discr); struct bfd_session *bfd_id_delete(uint32_t id); -struct bfd_session *bfd_key_delete(struct bfd_key key); +struct bfd_session *bfd_key_delete(struct bfd_key *key); struct sbfd_reflector *sbfd_discr_delete(uint32_t discr); bool bfd_id_insert(struct bfd_session *bs);