From 9a62e84b5b6379ff3419da8f52728b95f31bacf3 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Wed, 20 Sep 2017 00:02:01 -0300 Subject: [PATCH 1/4] zebra: fix logging of MPLS labels * use %u instead of %d, we don't want to print negative labels; * increase the size of label_buf to accommodate the worst case scenarios; * use strlcat() instead of strcat() as a security best practice. Signed-off-by: Renato Westphal --- zebra/rt_netlink.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index a939dde8d5..3b2d22fa95 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -845,7 +845,7 @@ static void _netlink_route_build_singlepath(const char *routedesc, int bytelen, { struct nexthop_label *nh_label; mpls_lse_t out_lse[MPLS_MAX_LABELS]; - char label_buf[100]; + char label_buf[256]; /* * label_buf is *only* currently used within debugging. @@ -876,12 +876,13 @@ static void _netlink_route_build_singlepath(const char *routedesc, int bytelen, 0, 0, bos); if (IS_ZEBRA_DEBUG_KERNEL) { if (!num_labels) - sprintf(label_buf, "label %d", + sprintf(label_buf, "label %u", nh_label->label[i]); else { - sprintf(label_buf1, "/%d", + sprintf(label_buf1, "/%u", nh_label->label[i]); - strcat(label_buf, label_buf1); + strlcat(label_buf, label_buf1, + sizeof(label_buf)); } } num_labels++; @@ -1044,7 +1045,7 @@ static void _netlink_route_build_multipath(const char *routedesc, int bytelen, { struct nexthop_label *nh_label; mpls_lse_t out_lse[MPLS_MAX_LABELS]; - char label_buf[100]; + char label_buf[256]; rtnh->rtnh_len = sizeof(*rtnh); rtnh->rtnh_flags = 0; @@ -1080,12 +1081,13 @@ static void _netlink_route_build_multipath(const char *routedesc, int bytelen, 0, 0, bos); if (IS_ZEBRA_DEBUG_KERNEL) { if (!num_labels) - sprintf(label_buf, "label %d", + sprintf(label_buf, "label %u", nh_label->label[i]); else { - sprintf(label_buf1, "/%d", + sprintf(label_buf1, "/%u", nh_label->label[i]); - strcat(label_buf, label_buf1); + strlcat(label_buf, label_buf1, + sizeof(label_buf)); } } num_labels++; From ad4527eb61d1d5908fc9c62c5538df89b2df2f28 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Wed, 20 Sep 2017 00:02:50 -0300 Subject: [PATCH 2/4] zebra: fix uninitialized prefixes in the handling of FEC messages This was causing some weird prefixes to pop up in my log files. One alternate solution would be to call apply_mask() on the prefix, but memcpy() is faster and just enough in this case. Signed-off-by: Renato Westphal --- zebra/zserv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zebra/zserv.c b/zebra/zserv.c index fd2c5dd97c..f888207818 100644 --- a/zebra/zserv.c +++ b/zebra/zserv.c @@ -825,6 +825,7 @@ static int zserv_fec_register(struct zserv *client, int sock, u_short length) while (l < length) { flags = stream_getw(s); + memset(&p, 0, sizeof(p)); p.family = stream_getw(s); if (p.family != AF_INET && p.family != AF_INET6) { zlog_err( @@ -875,6 +876,7 @@ static int zserv_fec_unregister(struct zserv *client, int sock, u_short length) while (l < length) { // flags = stream_getw(s); (void)stream_getw(s); + memset(&p, 0, sizeof(p)); p.family = stream_getw(s); if (p.family != AF_INET && p.family != AF_INET6) { zlog_err( From cbdb74116f01b4758880c38dbaccbba6fb9572e5 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Wed, 20 Sep 2017 00:04:04 -0300 Subject: [PATCH 3/4] bgpd: remove 'network' commands from the BGP_IPV6L node These commands don't belong in the BGP_IPV6L_NODE node anymore. A similar change was done for BGP_IPV4L_NODE in commit 9bedbb1e5. Signed-off-by: Renato Westphal --- bgpd/bgp_route.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 762519715c..0c2a2f6fe9 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -11464,12 +11464,6 @@ void bgp_route_init(void) install_element(BGP_IPV6M_NODE, &ipv6_bgp_network_cmd); install_element(BGP_IPV6M_NODE, &no_ipv6_bgp_network_cmd); - install_element(BGP_IPV6L_NODE, &bgp_table_map_cmd); - install_element(BGP_IPV6L_NODE, &ipv6_bgp_network_cmd); - install_element(BGP_IPV6L_NODE, &ipv6_bgp_network_route_map_cmd); - install_element(BGP_IPV6L_NODE, &no_bgp_table_map_cmd); - install_element(BGP_IPV6L_NODE, &no_ipv6_bgp_network_cmd); - install_element(BGP_NODE, &bgp_distance_cmd); install_element(BGP_NODE, &no_bgp_distance_cmd); install_element(BGP_NODE, &bgp_distance_source_cmd); From d855d11fadb8b0f105b177d980987ace503db9cf Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Wed, 20 Sep 2017 00:04:58 -0300 Subject: [PATCH 4/4] zebra: use a switch statement in nexthop_set_resolved() This makes the function much easier to read, and also faster. Signed-off-by: Renato Westphal --- zebra/zebra_rib.c | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 1a9bd5a58e..d46e0730ee 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -325,9 +325,11 @@ static void nexthop_set_resolved(afi_t afi, struct nexthop *newhop, resolved_hop = nexthop_new(); SET_FLAG(resolved_hop->flags, NEXTHOP_FLAG_ACTIVE); - /* If the resolving route specifies a gateway, use it */ - if (newhop->type == NEXTHOP_TYPE_IPV4 - || newhop->type == NEXTHOP_TYPE_IPV4_IFINDEX) { + + switch (newhop->type) { + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: + /* If the resolving route specifies a gateway, use it */ resolved_hop->type = newhop->type; resolved_hop->gate.ipv4 = newhop->gate.ipv4; @@ -337,9 +339,9 @@ static void nexthop_set_resolved(afi_t afi, struct nexthop *newhop, if (newhop->flags & NEXTHOP_FLAG_ONLINK) resolved_hop->flags |= NEXTHOP_FLAG_ONLINK; } - } - if (newhop->type == NEXTHOP_TYPE_IPV6 - || newhop->type == NEXTHOP_TYPE_IPV6_IFINDEX) { + break; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: resolved_hop->type = newhop->type; resolved_hop->gate.ipv6 = newhop->gate.ipv6; @@ -347,18 +349,17 @@ static void nexthop_set_resolved(afi_t afi, struct nexthop *newhop, resolved_hop->type = NEXTHOP_TYPE_IPV6_IFINDEX; resolved_hop->ifindex = newhop->ifindex; } - } - - /* If the resolving route is an interface route, - * it means the gateway we are looking up is connected - * to that interface. (The actual network is _not_ onlink). - * Therefore, the resolved route should have the original - * gateway as nexthop as it is directly connected. - * - * On Linux, we have to set the onlink netlink flag because - * otherwise, the kernel won't accept the route. - */ - if (newhop->type == NEXTHOP_TYPE_IFINDEX) { + break; + case NEXTHOP_TYPE_IFINDEX: + /* If the resolving route is an interface route, + * it means the gateway we are looking up is connected + * to that interface. (The actual network is _not_ onlink). + * Therefore, the resolved route should have the original + * gateway as nexthop as it is directly connected. + * + * On Linux, we have to set the onlink netlink flag because + * otherwise, the kernel won't accept the route. + */ resolved_hop->flags |= NEXTHOP_FLAG_ONLINK; if (afi == AFI_IP) { resolved_hop->type = NEXTHOP_TYPE_IPV4_IFINDEX; @@ -368,12 +369,13 @@ static void nexthop_set_resolved(afi_t afi, struct nexthop *newhop, resolved_hop->gate.ipv6 = nexthop->gate.ipv6; } resolved_hop->ifindex = newhop->ifindex; - } - - if (newhop->type == NEXTHOP_TYPE_BLACKHOLE) { + break; + case NEXTHOP_TYPE_BLACKHOLE: resolved_hop->type = NEXTHOP_TYPE_BLACKHOLE; resolved_hop->bh_type = nexthop->bh_type; + break; } + resolved_hop->rparent = nexthop; nexthop_add(&nexthop->resolved, resolved_hop); }