From 17cdd31e00b2ca2763c042a415bfcf03b614a783 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 6 Sep 2018 10:51:08 -0400 Subject: [PATCH] bgpd: Prevent possible crash when parsing v6 attributes The peer->nexthop.ifp pointer must be set when parsing the attributes in bgp_mp_reach_parse, notice this and fail gracefully. Rework bgp_nexthop_set to remove the HAVE_CUMULUS and to fail the nexthop_set when we have a zebra connection and no ifp pointer, as that not havinga zebra connection and no ifp pointer is legal. Signed-off-by: Donald Sharp --- bgpd/bgp_attr.c | 21 +++++++++++++++++++-- bgpd/bgp_network.c | 15 +++++++-------- bgpd/bgp_zebra.c | 31 ++++++++++++++++++++++++------- bgpd/bgp_zebra.h | 3 +++ bgpd/bgpd.h | 2 -- 5 files changed, 53 insertions(+), 19 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 6acd4c8cf..720afe6e0 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1707,8 +1707,14 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args, stream_getl(s); /* RD low */ } stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN); - if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) + if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) { + if (!peer->nexthop.ifp) { + zlog_warn("%s: interface not set appropriately to handle some attributes", + peer->host); + return BGP_ATTR_PARSE_WITHDRAW; + } attr->nh_ifindex = peer->nexthop.ifp->ifindex; + } break; case BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL: case BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL: @@ -1718,8 +1724,14 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args, stream_getl(s); /* RD low */ } stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN); - if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) + if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) { + if (!peer->nexthop.ifp) { + zlog_warn("%s: interface not set appropriately to handle some attributes", + peer->host); + return BGP_ATTR_PARSE_WITHDRAW; + } attr->nh_ifindex = peer->nexthop.ifp->ifindex; + } if (attr->mp_nexthop_len == BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL) { stream_getl(s); /* RD high */ @@ -1743,6 +1755,11 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args, attr->mp_nexthop_len = IPV6_MAX_BYTELEN; } + if (!peer->nexthop.ifp) { + zlog_warn("%s: Interface not set appropriately to handle this some attributes", + peer->host); + return BGP_ATTR_PARSE_WITHDRAW; + } attr->nh_lla_ifindex = peer->nexthop.ifp->ifindex; break; default: diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 22d5d35c8..f6cad6461 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -36,6 +36,7 @@ #include "filter.h" #include "ns.h" #include "lib_errors.h" +#include "nexthop.h" #include "bgpd/bgpd.h" #include "bgpd/bgp_open.h" @@ -44,6 +45,7 @@ #include "bgpd/bgp_debug.h" #include "bgpd/bgp_errors.h" #include "bgpd/bgp_network.h" +#include "bgpd/bgp_zebra.h" extern struct zebra_privs_t bgpd_privs; @@ -617,15 +619,12 @@ int bgp_getsockname(struct peer *peer) if (!peer->su_remote) return -1; - if (bgp_nexthop_set(peer->su_local, peer->su_remote, &peer->nexthop, - peer)) { -#if defined(HAVE_CUMULUS) - flog_err( - BGP_ERR_NH_UPD, - "%s: nexthop_set failed, resetting connection - intf %p", - peer->host, peer->nexthop.ifp); + if (!bgp_zebra_nexthop_set(peer->su_local, peer->su_remote, + &peer->nexthop, peer)) { + flog_err(BGP_ERR_NH_UPD, + "%s: nexthop_set failed, resetting connection - intf %p", + peer->host, peer->nexthop.ifp); return -1; -#endif } return 0; } diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 43afc317e..49fed1612 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -777,8 +777,9 @@ static int if_get_ipv4_address(struct interface *ifp, struct in_addr *addr) return 0; } -int bgp_nexthop_set(union sockunion *local, union sockunion *remote, - struct bgp_nexthop *nexthop, struct peer *peer) + +bool bgp_zebra_nexthop_set(union sockunion *local, union sockunion *remote, + struct bgp_nexthop *nexthop, struct peer *peer) { int ret = 0; struct interface *ifp = NULL; @@ -786,9 +787,9 @@ int bgp_nexthop_set(union sockunion *local, union sockunion *remote, memset(nexthop, 0, sizeof(struct bgp_nexthop)); if (!local) - return -1; + return false; if (!remote) - return -1; + return false; if (local->sa.sa_family == AF_INET) { nexthop->v4 = local->sin.sin_addr; @@ -815,8 +816,24 @@ int bgp_nexthop_set(union sockunion *local, union sockunion *remote, peer->bgp->vrf_id); } - if (!ifp) - return -1; + if (!ifp) { + /* + * BGP views do not currently get proper data + * from zebra( when attached ) to be able to + * properly resolve nexthops, so give this + * instance type a pass. + */ + if (peer->bgp->inst_type == BGP_INSTANCE_TYPE_VIEW) + return true; + /* + * If we have no interface data but we have established + * some connection w/ zebra than something has gone + * terribly terribly wrong here, so say this failed + * If we do not any zebra connection then not + * having a ifp pointer is ok. + */ + return zclient_num_connects ? false : true; + } nexthop->ifp = ifp; @@ -912,7 +929,7 @@ int bgp_nexthop_set(union sockunion *local, union sockunion *remote, /* If we have identified the local interface, there is no error for now. */ - return 0; + return true; } static struct in6_addr *bgp_info_to_ipv6_nexthop(struct bgp_info *info, diff --git a/bgpd/bgp_zebra.h b/bgpd/bgp_zebra.h index 546d72402..c00d9925e 100644 --- a/bgpd/bgp_zebra.h +++ b/bgpd/bgp_zebra.h @@ -73,6 +73,9 @@ extern int bgp_zebra_advertise_all_vni(struct bgp *, int); extern int bgp_zebra_num_connects(void); +extern bool bgp_zebra_nexthop_set(union sockunion *, union sockunion *, + struct bgp_nexthop *, struct peer *); + struct bgp_pbr_action; struct bgp_pbr_match; struct bgp_pbr_match_entry; diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 8a9974139..0a8962b4c 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1470,8 +1470,6 @@ extern void bgp_terminate(void); extern void bgp_reset(void); extern time_t bgp_clock(void); extern void bgp_zclient_reset(void); -extern int bgp_nexthop_set(union sockunion *, union sockunion *, - struct bgp_nexthop *, struct peer *); extern struct bgp *bgp_get_default(void); extern struct bgp *bgp_lookup(as_t, const char *); extern struct bgp *bgp_lookup_by_name(const char *); -- 2.39.5