From: Benoît Ganne Date: Thu, 16 Apr 2020 10:47:47 +0000 (+0200) Subject: igmp: fix igmp proxy group merge X-Git-Tag: v20.09-rc0~202 X-Git-Url: https://gerrit.fd.io/r/gitweb?p=vpp.git;a=commitdiff_plain;h=bd7f3422bbe38ba87888b765e94b56bfcbb9602c igmp: fix igmp proxy group merge When merging proxy groups in igmp_proxy_device_merge_group(), the call to igmp_proxy_device_merge_src() can end up removing the current proxy group via igmp_group_clear(). When that happens, it must returns NULL so that igmp_proxy_device_merge_config() does not send a IGMPv3 report for a dead proxy group. Make igmp_group_clear() reset the group pointer to NULL to fix this bug and to detect similar bugs more easily. Type: fix Change-Id: I229e55b5bfa71734d7844893f5209a66fa3cc8ae Signed-off-by: Benoît Ganne --- diff --git a/src/plugins/igmp/igmp.c b/src/plugins/igmp/igmp.c index 1e9f647cd11..eea39d33da2 100644 --- a/src/plugins/igmp/igmp.c +++ b/src/plugins/igmp/igmp.c @@ -282,7 +282,7 @@ igmp_listen (vlib_main_t * vm, } if (0 == igmp_group_n_srcs (group, mode)) - igmp_group_clear (group); + igmp_group_clear (&group); vec_free (added); vec_free (removed); diff --git a/src/plugins/igmp/igmp_config.c b/src/plugins/igmp/igmp_config.c index f9acc299ff1..7637adba5bf 100644 --- a/src/plugins/igmp/igmp_config.c +++ b/src/plugins/igmp/igmp_config.c @@ -31,7 +31,7 @@ igmp_clear_config (igmp_config_t * config) /* *INDENT-OFF* */ FOR_EACH_GROUP (group, config, ({ - igmp_group_clear (group); + igmp_group_clear (&group); })); /* *INDENT-ON* */ diff --git a/src/plugins/igmp/igmp_group.c b/src/plugins/igmp/igmp_group.c index 51a44d2dfd5..eec4c9b8f81 100644 --- a/src/plugins/igmp/igmp_group.c +++ b/src/plugins/igmp/igmp_group.c @@ -65,38 +65,39 @@ igmp_group_src_update (igmp_group_t * group, } void -igmp_group_clear (igmp_group_t * group) +igmp_group_clear (igmp_group_t ** group) { igmp_config_t *config; u32 ii; - ASSERT (group); + ASSERT (*group); - config = igmp_config_get (group->config); + config = igmp_config_get ((*group)->config); /* If interface is in ROUTER mode and IGMP proxy is enabled * remove mfib path. */ if (config->mode == IGMP_MODE_ROUTER) { - igmp_proxy_device_mfib_path_add_del (group, /* add */ 0); + igmp_proxy_device_mfib_path_add_del (*group, /* add */ 0); } IGMP_DBG ("clear-group: %U %U", - format_igmp_key, group->key, + format_igmp_key, (*group)->key, format_vnet_sw_if_index_name, vnet_get_main (), config->sw_if_index); - igmp_group_free_all_srcs (group); + igmp_group_free_all_srcs (*group); for (ii = 0; ii < IGMP_GROUP_N_TIMERS; ii++) { - igmp_timer_retire (&group->timers[ii]); + igmp_timer_retire (&(*group)->timers[ii]); } - hash_unset_mem (config->igmp_group_by_key, group->key); - clib_mem_free (group->key); - pool_put (igmp_main.groups, group); + hash_unset_mem (config->igmp_group_by_key, (*group)->key); + clib_mem_free ((*group)->key); + pool_put (igmp_main.groups, *group); + *group = 0; } igmp_group_t * diff --git a/src/plugins/igmp/igmp_group.h b/src/plugins/igmp/igmp_group.h index 7d4dfb6d243..0eb4f43ef05 100644 --- a/src/plugins/igmp/igmp_group.h +++ b/src/plugins/igmp/igmp_group.h @@ -103,7 +103,7 @@ do { \ */ struct igmp_config_t_; -extern void igmp_group_clear (igmp_group_t * group); +extern void igmp_group_clear (igmp_group_t ** group); extern void igmp_group_free_all_srcs (igmp_group_t * group); extern igmp_group_t *igmp_group_alloc (struct igmp_config_t_ *config, diff --git a/src/plugins/igmp/igmp_proxy.c b/src/plugins/igmp/igmp_proxy.c index dc2b4c13791..2167740fc8a 100644 --- a/src/plugins/igmp/igmp_proxy.c +++ b/src/plugins/igmp/igmp_proxy.c @@ -246,18 +246,18 @@ igmp_proxy_device_block_src (igmp_config_t * config, igmp_group_t * group, { igmp_proxy_device_mfib_path_add_del (proxy_group, 0); igmp_proxy_device_mfib_path_add_del (group, 0); - igmp_group_clear (proxy_group); + igmp_group_clear (&proxy_group); } } always_inline void -igmp_proxy_device_merge_src (igmp_group_t * proxy_group, igmp_src_t * src, +igmp_proxy_device_merge_src (igmp_group_t ** proxy_group, igmp_src_t * src, ip46_address_t ** srcaddrs, u8 block) { igmp_src_t *proxy_src; u32 d_config; - proxy_src = igmp_src_lookup (proxy_group, src->key); + proxy_src = igmp_src_lookup (*proxy_group, src->key); if (proxy_src == NULL) { @@ -267,11 +267,11 @@ igmp_proxy_device_merge_src (igmp_group_t * proxy_group, igmp_src_t * src, d_config = igmp_group_get (src->group)->config; proxy_src = - igmp_src_alloc (igmp_group_index (proxy_group), src->key, + igmp_src_alloc (igmp_group_index (*proxy_group), src->key, IGMP_MODE_HOST); - hash_set_mem (proxy_group->igmp_src_by_key - [proxy_group->router_filter_mode], proxy_src->key, + hash_set_mem ((*proxy_group)->igmp_src_by_key + [(*proxy_group)->router_filter_mode], proxy_src->key, igmp_src_index (proxy_src)); vec_validate_init_empty (proxy_src->referance_by_config_index, d_config, @@ -299,12 +299,12 @@ igmp_proxy_device_merge_src (igmp_group_t * proxy_group, igmp_src_t * src, vec_add1 (*srcaddrs, *proxy_src->key); - igmp_group_src_remove (proxy_group, proxy_src); + igmp_group_src_remove (*proxy_group, proxy_src); igmp_src_free (proxy_src); - if (igmp_group_n_srcs (proxy_group, IGMP_FILTER_MODE_INCLUDE) == 0) + if (igmp_group_n_srcs (*proxy_group, IGMP_FILTER_MODE_INCLUDE) == 0) { - igmp_proxy_device_mfib_path_add_del (proxy_group, 0); + igmp_proxy_device_mfib_path_add_del (*proxy_group, 0); igmp_group_clear (proxy_group); } return; @@ -348,7 +348,7 @@ igmp_proxy_device_merge_group (igmp_proxy_device_t * proxy_device, /* *INDENT-OFF* */ FOR_EACH_SRC (src, group, group->router_filter_mode, ({ - igmp_proxy_device_merge_src (proxy_group, src, srcaddrs, block); + igmp_proxy_device_merge_src (&proxy_group, src, srcaddrs, block); })); /* *INDENT-ON* */ return proxy_group; diff --git a/src/plugins/igmp/igmp_src.c b/src/plugins/igmp/igmp_src.c index 64768ab59bd..bec46f86399 100644 --- a/src/plugins/igmp/igmp_src.c +++ b/src/plugins/igmp/igmp_src.c @@ -66,7 +66,7 @@ igmp_src_exp (u32 obj, void *dat) igmp_src_free (src); if (0 == igmp_group_n_srcs (group, IGMP_FILTER_MODE_INCLUDE)) - igmp_group_clear (group); + igmp_group_clear (&group); } igmp_src_t *