*: fix segfault when sending more than MULTIPATH_NUM nexthops

This is a fallout from PR #1022 (zapi consolidation). In the early days,
the client daemons would allocate enough memory to send all nexthops
to zebra.  Then zebra would add all nexthops to the RIB and respect
MULTIPATH_NUM only when installing the routes in the kernel. Now things
are different and the client daemons can send at most MULTIPATH_NUM
nexthops to zebra, and failure to respect that will result in a buffer
overflow. The MULTIPATH_NUM limit in the new zebra API is a small price
we pay to avoid allocating memory for each route sent to zebra.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index cd52d28b208de043be01dfbf106a7015ee70ad2f..bc69b67de9e8d427a9975fe66d55488ede5c8ec2 100644 (file)
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -956,7 +956,7 @@ void bgp_zebra_announce(struct bgp_node *rn, struct prefix *p,
        struct zapi_route api;
        struct zapi_nexthop *api_nh;
        int nh_family;
-       int valid_nh_count = 0;
+       unsigned int valid_nh_count = 0;
        int has_valid_label = 0;
        u_char distance;
        struct peer *peer;
@@ -1010,6 +1010,9 @@ void bgp_zebra_announce(struct bgp_node *rn, struct prefix *p,
        /* Metric is currently based on the best-path only */
        metric = info->attr->med;
        for (mpinfo = info; mpinfo; mpinfo = bgp_info_mpath_next(mpinfo)) {
+               if (valid_nh_count >= multipath_num)
+                       break;
+
                *mpinfo_cp = *mpinfo;
 
                /* Get nexthop address-family */

diff --git a/bgpd/rfapi/vnc_zebra.c b/bgpd/rfapi/vnc_zebra.c
index d472e06fa5abdfa7c89fdad0c0dbb1cf97aa7958..3fc6ddfe35c27f75dcf1e8c81319fb889cf2c28f 100644 (file)
--- a/bgpd/rfapi/vnc_zebra.c
+++ b/bgpd/rfapi/vnc_zebra.c
@@ -368,8 +368,8 @@ static int vnc_zebra_read_route(int command, struct zclient *zclient,
 /*
  * low-level message builder
  */
-static void vnc_zebra_route_msg(struct prefix *p, int nhp_count, void *nhp_ary,
-                               int add) /* 1 = add, 0 = del */
+static void vnc_zebra_route_msg(struct prefix *p, unsigned int nhp_count,
+                               void *nhp_ary, int add) /* 1 = add, 0 = del */
 {
        struct zapi_route api;
        struct zapi_nexthop *api_nh;
@@ -389,8 +389,8 @@ static void vnc_zebra_route_msg(struct prefix *p, int nhp_count, void *nhp_ary,
 
        /* Nexthops */
        SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP);
-       api.nexthop_num = nhp_count;
-       for (i = 0; i < nhp_count; i++) {
+       api.nexthop_num = MIN(nhp_count, multipath_num);
+       for (i = 0; i < api.nexthop_num; i++) {
                struct in_addr *nhp_ary4;
                struct in6_addr *nhp_ary6;
 
@@ -426,7 +426,8 @@ static void vnc_zebra_route_msg(struct prefix *p, int nhp_count, void *nhp_ary,
 
 
 static void
-nve_list_to_nh_array(u_char family, struct list *nve_list, int *nh_count_ret,
+nve_list_to_nh_array(u_char family, struct list *nve_list,
+                    unsigned int *nh_count_ret,
                     void **nh_ary_ret,  /* returned address array */
                     void **nhp_ary_ret) /* returned pointer array */
 {
@@ -549,7 +550,7 @@ static void vnc_zebra_add_del_prefix(struct bgp *bgp,
 {
        struct list *nves;
 
-       int nexthop_count = 0;
+       unsigned int nexthop_count = 0;
        void *nh_ary = NULL;
        void *nhp_ary = NULL;
 
@@ -713,7 +714,7 @@ static void vnc_zebra_add_del_group_afi(struct bgp *bgp,
        uint8_t family = afi2family(afi);
 
        struct list *nves = NULL;
-       int nexthop_count = 0;
+       unsigned int nexthop_count = 0;
        void *nh_ary = NULL;
        void *nhp_ary = NULL;
 

diff --git a/eigrpd/eigrp_zebra.c b/eigrpd/eigrp_zebra.c
index e61b3d748c228bb49185e27bbe3a88a63cdf3c2d..95d97cf97f28aab9f2b61953ecc28c46ace387a6 100644 (file)
--- a/eigrpd/eigrp_zebra.c
+++ b/eigrpd/eigrp_zebra.c
@@ -379,10 +379,11 @@ void eigrp_zebra_route_add(struct prefix *p, struct list *successors)
        memcpy(&api.prefix, p, sizeof(*p));
 
        SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP);
-       api.nexthop_num = successors->count;
 
        /* Nexthop, ifindex, distance and metric information. */
        for (ALL_LIST_ELEMENTS_RO(successors, node, te)) {
+               if (count >= MULTIPATH_NUM)
+                       break;
                api_nh = &api.nexthops[count];
                if (te->adv_router->src.s_addr) {
                        api_nh->gate.ipv4 = te->adv_router->src;
@@ -393,6 +394,7 @@ void eigrp_zebra_route_add(struct prefix *p, struct list *successors)
 
                count++;
        }
+       api.nexthop_num = count;
 
        if (IS_DEBUG_EIGRP(zebra, ZEBRA_REDISTRIBUTE)) {
                char buf[2][PREFIX_STRLEN];

diff --git a/isisd/isis_zebra.c b/isisd/isis_zebra.c
index 99eb698b75f155777dd9307b1f8b9f073f52af1b..bc81314097f1429631cc3472641e3a3c1f24cf03 100644 (file)
--- a/isisd/isis_zebra.c
+++ b/isisd/isis_zebra.c
@@ -277,6 +277,8 @@ static void isis_zebra_route_add_route(struct prefix *prefix,
        case AF_INET:
                for (ALL_LIST_ELEMENTS_RO(route_info->nexthops, node,
                                          nexthop)) {
+                       if (count >= MULTIPATH_NUM)
+                               break;
                        api_nh = &api.nexthops[count];
                        /* FIXME: can it be ? */
                        if (nexthop->ip.s_addr != INADDR_ANY) {
@@ -292,6 +294,8 @@ static void isis_zebra_route_add_route(struct prefix *prefix,
        case AF_INET6:
                for (ALL_LIST_ELEMENTS_RO(route_info->nexthops6, node,
                                          nexthop6)) {
+                       if (count >= MULTIPATH_NUM)
+                               break;
                        if (!IN6_IS_ADDR_LINKLOCAL(&nexthop6->ip6)
                            && !IN6_IS_ADDR_UNSPECIFIED(&nexthop6->ip6)) {
                                continue;

diff --git a/lib/zclient.c b/lib/zclient.c
index 910e05cb47f8ee1ab8cffdd5123ab0dabe65145e..e30f166ddeab256fc61458c20d28bcba539ab9df 100644 (file)
--- a/lib/zclient.c
+++ b/lib/zclient.c
@@ -915,9 +915,10 @@ int zapi_route_encode(u_char cmd, struct stream *s, struct zapi_route *api)
 
                        prefix2str(&api->prefix, buf, sizeof(buf));
                        zlog_warn(
-                               "%s: prefix %s: encoding %u nexthops out of %u",
-                               __func__, buf, MULTIPATH_NUM, api->nexthop_num);
-                       api->nexthop_num = MULTIPATH_NUM;
+                               "%s: prefix %s: can't encode %u nexthops "
+                               "(maximum is %u)",
+                               __func__, buf, api->nexthop_num, MULTIPATH_NUM);
+                       return -1;
                }
 
                stream_putw(s, api->nexthop_num);

diff --git a/ospf6d/ospf6_zebra.c b/ospf6d/ospf6_zebra.c
index 2372bc54b798aa3b159e167b31972a980f004c2f..30bb4393c7e1a820f02ed09d7c254c86109043e3 100644 (file)
--- a/ospf6d/ospf6_zebra.c
+++ b/ospf6d/ospf6_zebra.c
@@ -341,8 +341,8 @@ static void ospf6_zebra_route_update(int type, struct ospf6_route *request)
        api.safi = SAFI_UNICAST;
        api.prefix = *dest;
        SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP);
-       api.nexthop_num = nhcount;
-       ospf6_route_zebra_copy_nexthops(request, api.nexthops, nhcount);
+       api.nexthop_num = MIN(nhcount, MULTIPATH_NUM);
+       ospf6_route_zebra_copy_nexthops(request, api.nexthops, api.nexthop_num);
        SET_FLAG(api.message, ZAPI_MESSAGE_METRIC);
        api.metric = (request->path.metric_type == 2 ? request->path.u.cost_e2
                                                     : request->path.cost);

diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c
index e26a33c35f1e6c5dbb74c9a235569226c1135014..9bba2c980658b1ef0b37bd52778779dd87fbf7f6 100644 (file)
--- a/ospfd/ospf_zebra.c
+++ b/ospfd/ospf_zebra.c
@@ -349,7 +349,6 @@ void ospf_zebra_add(struct prefix_ipv4 *p, struct ospf_route * or)
 
        memcpy(&api.prefix, p, sizeof(*p));
        SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP);
-       api.nexthop_num = or->paths->count;
 
        /* Metric value. */
        SET_FLAG(api.message, ZAPI_MESSAGE_METRIC);
@@ -377,6 +376,8 @@ void ospf_zebra_add(struct prefix_ipv4 *p, struct ospf_route * or)
 
        /* Nexthop, ifindex, distance and metric information. */
        for (ALL_LIST_ELEMENTS_RO(or->paths, node, path)) {
+               if (count >= MULTIPATH_NUM)
+                       break;
                api_nh = &api.nexthops[count];
 #ifdef HAVE_NETLINK
                if (path->unnumbered || (path->nexthop.s_addr != INADDR_ANY
@@ -407,6 +408,7 @@ void ospf_zebra_add(struct prefix_ipv4 *p, struct ospf_route * or)
                                path->ifindex);
                }
        }
+       api.nexthop_num = count;
 
        zclient_route_send(ZEBRA_ROUTE_ADD, zclient, &api);
 }

diff --git a/ripd/rip_zebra.c b/ripd/rip_zebra.c
index 2140e8b1101df96748c04e6e44604cda0c3269e5..28144a24351dd2aaf83233d949fb8e14f9bb22ea 100644 (file)
--- a/ripd/rip_zebra.c
+++ b/ripd/rip_zebra.c
@@ -53,6 +53,8 @@ static void rip_zebra_ipv4_send(struct route_node *rp, u_char cmd)
 
        SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP);
        for (ALL_LIST_ELEMENTS_RO(list, listnode, rinfo)) {
+               if (count >= MULTIPATH_NUM)
+                       break;
                api_nh = &api.nexthops[count];
                api_nh->gate.ipv4 = rinfo->nexthop;
                api_nh->type = NEXTHOP_TYPE_IPV4;

diff --git a/ripngd/ripng_zebra.c b/ripngd/ripng_zebra.c
index 283d8691a3db243ffc4596a4c243b545d03df8c6..7edaaa5dff6d165371f079412130c9da5e6f4167 100644 (file)
--- a/ripngd/ripng_zebra.c
+++ b/ripngd/ripng_zebra.c
@@ -54,6 +54,8 @@ static void ripng_zebra_ipv6_send(struct route_node *rp, u_char cmd)
 
        SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP);
        for (ALL_LIST_ELEMENTS_RO(list, listnode, rinfo)) {
+               if (count >= MULTIPATH_NUM)
+                       break;
                api_nh = &api.nexthops[count];
                api_nh->gate.ipv6 = rinfo->nexthop;
                api_nh->ifindex = rinfo->ifindex;