From af734bc7cf38df4b17896c2767b5d5e967c6dd8d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 18 Mar 2018 21:21:16 -0400 Subject: [PATCH 1/5] zebra: Fix leaked fd. When we detect an error condition, close down the opened fd. Signed-off-by: Donald Sharp Date: Sun, 18 Mar 2018 21:30:27 -0400 Subject: [PATCH 2/5] zebra: Compare to the number of elements not size of array When figuring out whom to call and if we actually can legally call into the handler array actually use the number of elements in the array instead of the size of the array. Signed-off-by: Donald Sharp --- zebra/zserv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra/zserv.c b/zebra/zserv.c index 57b1be4b8b..5b279a7cb2 100644 --- a/zebra/zserv.c +++ b/zebra/zserv.c @@ -2614,7 +2614,7 @@ static inline void zserv_handle_commands(struct zserv *client, struct stream *msg, struct zebra_vrf *zvrf) { - if (hdr->command > sizeof(zserv_handlers) + if (hdr->command > array_size(zserv_handlers) || zserv_handlers[hdr->command] == NULL) zlog_info("Zebra received unknown command %d", hdr->command); else From 993b1432259e1859dc41c73e345bc573968721c5 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 18 Mar 2018 21:46:58 -0400 Subject: [PATCH 3/5] pimd: Fix leaked fd and prevent null pointer deref When the pim_nexthop_lookup fails, close the opened fd as part of the failure condition. Additionally pim_nexthop_lookup assumes that we've actually already looked up a nexthop in the past. Signed-off-by: Donald Sharp --- pimd/pim_igmp_mtrace.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pimd/pim_igmp_mtrace.c b/pimd/pim_igmp_mtrace.c index 5e2e316d85..9e59dc31b6 100644 --- a/pimd/pim_igmp_mtrace.c +++ b/pimd/pim_igmp_mtrace.c @@ -256,9 +256,11 @@ static int mtrace_un_forward_packet(struct pim_instance *pim, struct ip *ip_hdr, pim_socket_ip_hdr(fd); if (interface == NULL) { + memset(&nexthop, 0, sizeof(nexthop)); ret = pim_nexthop_lookup(pim, &nexthop, ip_hdr->ip_dst, 0); if (ret != 0) { + close(fd); if (PIM_DEBUG_MTRACE) zlog_warn( "Dropping mtrace packet, " @@ -434,6 +436,7 @@ static int mtrace_send_response(struct pim_instance *pim, if (PIM_DEBUG_MTRACE) zlog_debug("mtrace response to RP"); } else { + memset(&nexthop, 0, sizeof(nexthop)); /* TODO: should use unicast rib lookup */ ret = pim_nexthop_lookup(pim, &nexthop, mtracep->rsp_addr, 1); @@ -613,6 +616,7 @@ int igmp_mtrace_recv_qry_req(struct igmp_sock *igmp, struct ip *ip_hdr, nh_addr.s_addr = 0; + memset(&nexthop, 0, sizeof(nexthop)); ret = pim_nexthop_lookup(pim, &nexthop, mtracep->src_addr, 1); if (ret == 0) { From 978caa0c88bad88a78d4f0b3b3d493522693ba65 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 18 Mar 2018 22:02:24 -0400 Subject: [PATCH 4/5] zebra: Free memory leak Free the memory leaked stream in failure cases. Signed-off-by: Donald Sharp --- zebra/zserv.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/zebra/zserv.c b/zebra/zserv.c index 5b279a7cb2..d0aab728f2 100644 --- a/zebra/zserv.c +++ b/zebra/zserv.c @@ -222,11 +222,15 @@ int zsend_interface_link_params(struct zserv *client, struct interface *ifp) struct stream *s = stream_new(ZEBRA_MAX_PACKET_SIZ); /* Check this client need interface information. */ - if (!client->ifinfo) + if (!client->ifinfo) { + stream_free(s); return 0; + } - if (!ifp->link_params) + if (!ifp->link_params) { + stream_free(s); return 0; + } zclient_create_header(s, ZEBRA_INTERFACE_LINK_PARAMS, ifp->vrf_id); @@ -234,8 +238,10 @@ int zsend_interface_link_params(struct zserv *client, struct interface *ifp) stream_putl(s, ifp->ifindex); /* Then TE Link Parameters */ - if (zebra_interface_link_params_write(s, ifp) == 0) + if (zebra_interface_link_params_write(s, ifp) == 0) { + stream_free(s); return 0; + } /* Write packet size. */ stream_putw_at(s, 0, stream_get_endp(s)); @@ -595,8 +601,10 @@ int zsend_redistribute_route(int cmd, struct zserv *client, struct prefix *p, struct stream *s = stream_new(ZEBRA_MAX_PACKET_SIZ); /* Encode route and send. */ - if (zapi_route_encode(cmd, s, &api) < 0) + if (zapi_route_encode(cmd, s, &api) < 0) { + stream_free(s); return -1; + } if (IS_ZEBRA_DEBUG_SEND) { char buf_prefix[PREFIX_STRLEN]; From c3e345b1d5e6914961eb7b1c04f2fb26d70a5215 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 18 Mar 2018 22:12:45 -0400 Subject: [PATCH 5/5] bgpd: Don't leak the ecommunity_ecom2str string in debug Signed-off-by: Donald Sharp --- bgpd/bgp_mplsvpn.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index d87f78a783..3096afc4c8 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -627,14 +627,12 @@ void vpn_leak_from_vrf_update(struct bgp *bgp_vpn, /* to */ &static_attr); /* hashed refcounted everything */ bgp_attr_flush(&static_attr); /* free locally-allocated parts */ - if (debug) { - const char *s = ""; + if (debug && new_attr->ecommunity) { + char *s = ecommunity_ecom2str(new_attr->ecommunity, + ECOMMUNITY_FORMAT_ROUTE_MAP, 0); - if (new_attr->ecommunity) { - s = ecommunity_ecom2str(new_attr->ecommunity, - ECOMMUNITY_FORMAT_ROUTE_MAP, 0); - } zlog_debug("%s: new_attr->ecommunity{%s}", __func__, s); + XFREE(MTYPE_ECOMMUNITY_STR, s); } /* Now new_attr is an allocated interned attr */