From 0eaa6523f6620a8ab2db0df6879e4b64dd27a6fe Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 19 May 2023 09:54:05 -0400 Subject: [PATCH 1/3] zebra: Do not allow old FPM to access freed memory after shutdown On shutdown, the old FPM queues up dests to be sent to the FPM listener. This is done through the rib_shutdown hook. Which is called when the table that the routes are stored in are being deleted. This dest has pointers to the rnode. The rnode has pointers to the table it is associated with as well as the table->info pointer for the zebra data associated with this table. The FPM after this attempts to tell this to it's listener via events. Unfortunately the zvrf, table_id and nl_pid was being grabbed from memory that had been freed! Since all this can be grabbed from memory that has not been freed on shutdown let's switch over to using that instead of freed memory for gathering data. Signed-off-by: Donald Sharp --- zebra/zebra_fpm_netlink.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/zebra/zebra_fpm_netlink.c b/zebra/zebra_fpm_netlink.c index 5adca14d71..ba34951e76 100644 --- a/zebra/zebra_fpm_netlink.c +++ b/zebra/zebra_fpm_netlink.c @@ -252,20 +252,15 @@ static int netlink_route_info_fill(struct netlink_route_info *ri, int cmd, rib_dest_t *dest, struct route_entry *re) { struct nexthop *nexthop; - struct rib_table_info *table_info = - rib_table_info(rib_dest_table(dest)); - struct zebra_vrf *zvrf = table_info->zvrf; memset(ri, 0, sizeof(*ri)); ri->prefix = rib_dest_prefix(dest); ri->af = rib_dest_af(dest); - if (zvrf && zvrf->zns) - ri->nlmsg_pid = zvrf->zns->netlink_dplane_out.snl.nl_pid; + ri->nlmsg_pid = pid; ri->nlmsg_type = cmd; - ri->rtm_table = table_info->table_id; ri->rtm_protocol = RTPROT_UNSPEC; /* @@ -280,6 +275,8 @@ static int netlink_route_info_fill(struct netlink_route_info *ri, int cmd, return 0; } + ri->rtm_table = re->table; + ri->rtm_protocol = netlink_proto_from_route_type(re->type); ri->rtm_type = RTN_UNICAST; ri->metric = &re->metric; From 540334324c59e3ae92622ba211b073488e477460 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 18 May 2023 15:59:43 -0400 Subject: [PATCH 2/3] zebra: Properly handle zfpm_g->t_conn_down in zebra_fpm.c The t_conn_down pointer was being set to NULL when it already was. The t_conn_down pointer was being dropped( and leaving a thread possibly running in the background ) which could cause problems on shutdown. And finally when shutting down the t_conn_down event was not being stopped at all. Signed-off-by: Donald Sharp --- zebra/zebra_fpm.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/zebra/zebra_fpm.c b/zebra/zebra_fpm.c index e379a5868c..4c2947cfd3 100644 --- a/zebra/zebra_fpm.c +++ b/zebra/zebra_fpm.c @@ -497,6 +497,11 @@ static inline void zfpm_connect_off(void) EVENT_OFF(zfpm_g->t_connect); } +static inline void zfpm_conn_down_off(void) +{ + EVENT_OFF(zfpm_g->t_conn_down); +} + /* * zfpm_conn_up_thread_cb * @@ -635,8 +640,6 @@ static void zfpm_conn_down_thread_cb(struct event *thread) while ((mac = TAILQ_FIRST(&zfpm_g->mac_q)) != NULL) zfpm_mac_info_del(mac); - zfpm_g->t_conn_down = NULL; - iter = &zfpm_g->t_conn_down_state.iter; while ((rnode = zfpm_rnodes_iter_next(iter))) { @@ -667,7 +670,6 @@ static void zfpm_conn_down_thread_cb(struct event *thread) zfpm_g->stats.t_conn_down_yields++; zfpm_rnodes_iter_pause(iter); - zfpm_g->t_conn_down = NULL; event_add_timer_msec(zfpm_g->master, zfpm_conn_down_thread_cb, NULL, 0, &zfpm_g->t_conn_down); return; @@ -712,7 +714,7 @@ static void zfpm_connection_down(const char *detail) */ assert(!zfpm_g->t_conn_down); zfpm_rnodes_iter_init(&zfpm_g->t_conn_down_state.iter); - zfpm_g->t_conn_down = NULL; + zfpm_conn_down_off(); event_add_timer_msec(zfpm_g->master, zfpm_conn_down_thread_cb, NULL, 0, &zfpm_g->t_conn_down); zfpm_g->stats.t_conn_down_starts++; @@ -2042,6 +2044,7 @@ static int zfpm_fini(void) zfpm_write_off(); zfpm_read_off(); zfpm_connect_off(); + zfpm_conn_down_off(); zfpm_stop_stats_timer(); From 5ec001aa53827236cd194ef631e59d04b429ac30 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 18 May 2023 16:03:01 -0400 Subject: [PATCH 3/3] zebra: On shutdown stop hook calls for fpm rmac updates When shutting down zebra, the hook for the rmac update was not being unregistered. As such it would be possible to get into a condition where more rmacs are being added to the queue for handling in the future after we are told to shutdown. Signed-off-by: Donald Sharp --- zebra/zebra_fpm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zebra/zebra_fpm.c b/zebra/zebra_fpm.c index 4c2947cfd3..699f3ed110 100644 --- a/zebra/zebra_fpm.c +++ b/zebra/zebra_fpm.c @@ -2049,6 +2049,8 @@ static int zfpm_fini(void) zfpm_stop_stats_timer(); hook_unregister(rib_update, zfpm_trigger_update); + hook_unregister(zebra_rmac_update, zfpm_trigger_rmac_update); + return 0; }