From c8b891b48392e0d29eb7b56942e4df697d4c14fd Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Mon, 6 Jan 2020 13:33:45 -0500 Subject: [PATCH] zebra: reset nexthop pointer in zread of nexthops We were not resetting the nexthop pointer to NULL for each new read of a nexthop from the zapi route. On the chance we get a nexthop that does not have a proper type, we will not create a new nexthop and update that pointer, thus it still has the last valid one and will create a group with two pointers to the same nexthop. Then when it enters any code that iterates the group, it loops endlessly. This was found with zapi fuzzing. ``` 0x00007f728891f1c3 in jhash2 (k=, length=, initval=12183506) at lib/jhash.c:138 0x00007f728896d92c in nexthop_hash (nexthop=) at lib/nexthop.c:563 0x00007f7288979ece in nexthop_group_hash (nhg=) at lib/nexthop_group.c:394 0x0000000000621036 in zebra_nhg_hash_key (arg=) at zebra/zebra_nhg.c:356 0x00007f72888ec0e1 in hash_get (hash=, data=0x7ffffb94aef0, alloc_func=0x0) at lib/hash.c:138 0x00007f72888ee118 in hash_lookup (hash=0x7f7288de2f10, data=0x7f728908e7fc) at lib/hash.c:183 0x0000000000626613 in zebra_nhg_find (nhe=0x7ffffb94b080, id=0, nhg=0x6020000032d0, nhg_depends=0x0, vrf_id=, afi=, type=) at zebra/zebra_nhg.c:541 0x0000000000625f39 in zebra_nhg_rib_find (id=0, nhg=, rt_afi=AFI_IP) at zebra/zebra_nhg.c:1126 0x000000000065f953 in rib_add_multipath (afi=AFI_IP, safi=, p=0x7ffffb94b370, src_p=0x0, re=0x6070000013d0, ng=0x7f728908e7fc) at zebra/zebra_rib.c:2616 0x0000000000768f90 in zread_route_add (client=0x61f000000080, hdr=, msg=, zvrf=) at zebra/zapi_msg.c:1596 0x000000000077c135 in zserv_handle_commands (client=, msg=0x61b000000780) at zebra/zapi_msg.c:2636 0x0000000000575e1f in main (argc=, argv=) at zebra/main.c:309 ``` ``` (gdb) p *nhg->nexthop $4 = {next = 0x5488e0, prev = 0x5488e0, vrf_id = 16843009, ifindex = 16843009, type = NEXTHOP_TYPE_IFINDEX, flags = 8 '\b', {gate = {ipv4 = {s_addr = 0}, ipv6 = {__in6_u = {__u6_addr8 = '\000' , __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}}, bh_type = BLACKHOLE_UNSPEC}, src = {ipv4 = {s_addr = 0}, ipv6 = {__in6_u = {__u6_addr8 = '\000' , __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}}, rmap_src = {ipv4 = {s_addr = 0}, ipv6 = {__in6_u = {__u6_addr8 = '\000' , __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}}, resolved = 0x0, rparent = 0x0, nh_label_type = ZEBRA_LSP_NONE, nh_label = 0x0, weight = 1 '\001'} (gdb) quit ``` Signed-off-by: Stephen Worley --- zebra/zapi_msg.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index c21d00bbe6..4fa7a3c164 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1475,6 +1475,8 @@ static void zread_route_add(ZAPI_HANDLER_ARGS) api_nh = &api.nexthops[i]; ifindex_t ifindex = 0; + nexthop = NULL; + if (IS_ZEBRA_DEBUG_RECV) zlog_debug("nh type %d", api_nh->type); -- 2.39.5