From f2de2bfea184034f675ab4a521e7deaae58c5ff8 Mon Sep 17 00:00:00 2001 From: Alberto Compagno Date: Tue, 14 Jan 2020 12:02:48 +0100 Subject: [HICN-477] Fixed strategy get ctx that could lead to a segfault Change-Id: Ic0d4f5a6919cb68255e788ac288d17492a6570a5 Signed-off-by: Alberto Compagno --- hicn-plugin/src/strategies/dpo_rr.c | 141 ++++++++++++++++-------------------- 1 file changed, 61 insertions(+), 80 deletions(-) (limited to 'hicn-plugin/src/strategies/dpo_rr.c') diff --git a/hicn-plugin/src/strategies/dpo_rr.c b/hicn-plugin/src/strategies/dpo_rr.c index 447a25c52..4cddd513c 100644 --- a/hicn-plugin/src/strategies/dpo_rr.c +++ b/hicn-plugin/src/strategies/dpo_rr.c @@ -95,10 +95,11 @@ hicn_dpo_strategy_rr_get_type (void) void hicn_strategy_rr_ctx_lock (dpo_id_t * dpo) { - if (dpo->dpoi_index != 0) + hicn_strategy_rr_ctx_t *hicn_strategy_rr_ctx = + (hicn_strategy_rr_ctx_t *) hicn_strategy_rr_ctx_get (dpo->dpoi_index); + + if (hicn_strategy_rr_ctx != NULL) { - hicn_strategy_rr_ctx_t *hicn_strategy_rr_ctx = - (hicn_strategy_rr_ctx_t *) hicn_strategy_rr_ctx_get (dpo->dpoi_index); hicn_strategy_rr_ctx->default_ctx.locks++; } } @@ -106,10 +107,10 @@ hicn_strategy_rr_ctx_lock (dpo_id_t * dpo) void hicn_strategy_rr_ctx_unlock (dpo_id_t * dpo) { - if (dpo->dpoi_index != 0) + hicn_strategy_rr_ctx_t *hicn_strategy_rr_ctx = + (hicn_strategy_rr_ctx_t *) hicn_strategy_rr_ctx_get (dpo->dpoi_index); + if (hicn_strategy_rr_ctx != NULL) { - hicn_strategy_rr_ctx_t *hicn_strategy_rr_ctx = - (hicn_strategy_rr_ctx_t *) hicn_strategy_rr_ctx_get (dpo->dpoi_index); hicn_strategy_rr_ctx->default_ctx.locks--; if (0 == hicn_strategy_rr_ctx->default_ctx.locks) @@ -135,7 +136,7 @@ format_hicn_strategy_rr_ctx (u8 * s, va_list * ap) format (s, "hicn-rr, next hop Face %d", dpo->default_ctx.next_hops[dpo->current_nhop].dpoi_index); - for (i = 0; i < HICN_PARAM_FIB_ENTRY_NHOPS_MAX; i++) + for (i = 0; i < HICN_PARAM_FIB_ENTRY_NHOPS_MAX && dpo != NULL; i++) { u8 *buf = NULL; if (i < dpo->default_ctx.entry_count) @@ -201,7 +202,7 @@ hicn_strategy_rr_ctx_get (index_t index) hicn_strategy_rr_ctx = (pool_elt_at_index (hicn_strategy_rr_ctx_pool, index)); } - return &hicn_strategy_rr_ctx->default_ctx; + return (hicn_dpo_ctx_t *)hicn_strategy_rr_ctx; } int @@ -210,46 +211,43 @@ hicn_strategy_rr_ctx_add_nh (const dpo_id_t * nh, index_t dpo_idx) hicn_strategy_rr_ctx_t *hicn_strategy_rr_ctx = (hicn_strategy_rr_ctx_t *) hicn_strategy_rr_ctx_get (dpo_idx); - if (hicn_strategy_rr_ctx != NULL) + if (hicn_strategy_rr_ctx == NULL) { + return HICN_ERROR_STRATEGY_NOT_FOUND; + } - int empty = hicn_strategy_rr_ctx->default_ctx.entry_count; - - /* Iterate through the list of faces to add new faces */ - for (int i = 0; i < hicn_strategy_rr_ctx->default_ctx.entry_count; i++) - { - if (!memcmp - (nh, &hicn_strategy_rr_ctx->default_ctx.next_hops[i], - sizeof (dpo_id_t))) - { - /* If face is marked as deleted, ignore it */ - hicn_face_t *face = - hicn_dpoi_get_from_idx (hicn_strategy_rr_ctx->default_ctx. - next_hops[i].dpoi_index); - if (face->shared.flags & HICN_FACE_FLAGS_DELETED) - { - continue; - } - return HICN_ERROR_DPO_CTX_NHOPS_EXISTS; - } - } + int empty = hicn_strategy_rr_ctx->default_ctx.entry_count; - /* Get an empty place */ - if (empty > HICN_PARAM_FIB_ENTRY_NHOPS_MAX) - { - return HICN_ERROR_DPO_CTX_NHOPS_NS; - } - if (PREDICT_FALSE (empty > HICN_PARAM_FIB_ENTRY_NHOPS_MAX)) - { - return HICN_ERROR_DPO_CTX_NHOPS_NS; - } - clib_memcpy (&hicn_strategy_rr_ctx->default_ctx.next_hops[empty], nh, - sizeof (dpo_id_t)); - hicn_strategy_rr_ctx->default_ctx.entry_count++; + /* Iterate through the list of faces to add new faces */ + for (int i = 0; i < hicn_strategy_rr_ctx->default_ctx.entry_count; i++) + { + if (!memcmp + (nh, &hicn_strategy_rr_ctx->default_ctx.next_hops[i], + sizeof (dpo_id_t))) + { + /* If face is marked as deleted, ignore it */ + hicn_face_t *face = + hicn_dpoi_get_from_idx (hicn_strategy_rr_ctx->default_ctx. + next_hops[i].dpoi_index); + if (face->shared.flags & HICN_FACE_FLAGS_DELETED) + { + continue; + } + return HICN_ERROR_DPO_CTX_NHOPS_EXISTS; + } + } - return HICN_ERROR_NONE; + /* Get an empty place */ + if (empty > HICN_PARAM_FIB_ENTRY_NHOPS_MAX) + { + return HICN_ERROR_DPO_CTX_NHOPS_NS; } - return HICN_ERROR_DPO_CTX_NOT_FOUND; + + clib_memcpy (&hicn_strategy_rr_ctx->default_ctx.next_hops[empty], nh, + sizeof (dpo_id_t)); + hicn_strategy_rr_ctx->default_ctx.entry_count++; + + return HICN_ERROR_NONE; } int @@ -258,52 +256,35 @@ hicn_strategy_rr_ctx_del_nh (hicn_face_id_t face_id, index_t dpo_idx, { hicn_strategy_rr_ctx_t *hicn_strategy_rr_ctx = (hicn_strategy_rr_ctx_t *) hicn_strategy_rr_ctx_get (dpo_idx); - int ret = HICN_ERROR_NONE; - int nh_id = ~0; + int ret = HICN_ERROR_DPO_CTX_NOT_FOUND; dpo_id_t invalid = NEXT_HOP_INVALID; - if (hicn_strategy_rr_ctx != NULL) + if (hicn_strategy_rr_ctx == NULL) { - for (int i = 0; i < hicn_strategy_rr_ctx->default_ctx.entry_count; i++) - { - if (hicn_strategy_rr_ctx->default_ctx.next_hops[i].dpoi_index == - face_id) - { - nh_id = i; - hicn_face_unlock (&hicn_strategy_rr_ctx-> - default_ctx.next_hops[i]); - hicn_strategy_rr_ctx->default_ctx.next_hops[i] = invalid; - hicn_strategy_rr_ctx->default_ctx.entry_count--; - } - } - - if (0 == hicn_strategy_rr_ctx->default_ctx.entry_count) - { - fib_table_entry_special_remove (HICN_FIB_TABLE, fib_pfx, - FIB_SOURCE_PLUGIN_HI); - } + return HICN_ERROR_STRATEGY_NOT_FOUND; } - else + + for (int i = 0; i < hicn_strategy_rr_ctx->default_ctx.entry_count; i++) { - ret = HICN_ERROR_DPO_CTX_NOT_FOUND; + if (hicn_strategy_rr_ctx->default_ctx.next_hops[i].dpoi_index == + face_id) + { + hicn_face_unlock (&hicn_strategy_rr_ctx-> + default_ctx.next_hops[i]); + hicn_strategy_rr_ctx->default_ctx.entry_count--; + hicn_strategy_rr_ctx->default_ctx.next_hops[i] = hicn_strategy_rr_ctx->default_ctx.next_hops[hicn_strategy_rr_ctx->default_ctx.entry_count]; + hicn_strategy_rr_ctx->default_ctx.next_hops[hicn_strategy_rr_ctx->default_ctx.entry_count] = invalid; + ret = HICN_ERROR_NONE; + break; + } } - /* - * Remove any possible hole in the arrays of dpos - */ - if (hicn_strategy_rr_ctx->default_ctx.entry_count > 0 && nh_id != ~0 - && nh_id < hicn_strategy_rr_ctx->default_ctx.entry_count - 1) + if (0 == hicn_strategy_rr_ctx->default_ctx.entry_count) { - int i; - for (i = nh_id; i < hicn_strategy_rr_ctx->default_ctx.entry_count; i++) - { - clib_memcpy (&hicn_strategy_rr_ctx->default_ctx.next_hops[i], - &hicn_strategy_rr_ctx->default_ctx.next_hops[i + 1], - sizeof (dpo_id_t)); - } - /* Set as invalid the last dpo */ - hicn_strategy_rr_ctx->default_ctx.next_hops[i] = invalid; + fib_table_entry_special_remove (HICN_FIB_TABLE, fib_pfx, + FIB_SOURCE_PLUGIN_HI); } + return ret; } -- cgit 1.2.3-korg