From 778cb26fbd058cc6ddb5496610754547060e2133 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 11 May 2016 20:22:45 -0400 Subject: [PATCH] Fix non initialized usage of data in zebra_rnh.c In zebra_deregister_rnh_static_nexthops the nh_p structure was not being properly initialized for all the cases that we could be storing a nexthop for. This was causing code later to retrieve the table from an nh_p->family which was garbage. In the case of BLACKHOLE and Ifindex based routes do nothing because they shouldn't be a nexthop considered for NHT. ==2239== Conditional jump or move depends on uninitialised value(s) ==2239== at 0x4E5F6CE: family2afi (prefix.c:217) ==2239== by 0x155F7C: get_rnh_table (zebra_rnh.c:83) ==2239== by 0x156194: zebra_lookup_rnh (zebra_rnh.c:148) ==2239== by 0x15655E: zebra_deregister_rnh_static_nh (zebra_rnh.c:242) ==2239== by 0x156681: zebra_deregister_rnh_static_nexthops (zebra_rnh.c:280) ==2239== by 0x12F3DF: rib_unlink (zebra_rib.c:2210) ==2239== by 0x12E9CE: rib_process (zebra_rib.c:1843) ==2239== by 0x12EA8A: process_subq (zebra_rib.c:1873) ==2239== by 0x12ECAF: meta_queue_process (zebra_rib.c:1936) ==2239== by 0x4E89625: work_queue_run (workqueue.c:298) ==2239== by 0x4E63230: thread_call (thread.c:1577) ==2239== by 0x125830: main (main.c:432) ==2239== ==2239== Conditional jump or move depends on uninitialised value(s) ==2239== at 0x4E5F6DB: family2afi (prefix.c:220) ==2239== by 0x155F7C: get_rnh_table (zebra_rnh.c:83) ==2239== by 0x156194: zebra_lookup_rnh (zebra_rnh.c:148) ==2239== by 0x15655E: zebra_deregister_rnh_static_nh (zebra_rnh.c:242) ==2239== by 0x156681: zebra_deregister_rnh_static_nexthops (zebra_rnh.c:280) ==2239== by 0x12F3DF: rib_unlink (zebra_rib.c:2210) ==2239== by 0x12E9CE: rib_process (zebra_rib.c:1843) ==2239== by 0x12EA8A: process_subq (zebra_rib.c:1873) ==2239== by 0x12ECAF: meta_queue_process (zebra_rib.c:1936) ==2239== by 0x4E89625: work_queue_run (workqueue.c:298) ==2239== by 0x4E63230: thread_call (thread.c:1577) ==2239== by 0x125830: main (main.c:432) Ticket: CM-10667 Signed-off-by: Donald Sharp Reviewed-by: Vivek Venkatraman Reviewed-by: Shrijeet Mukherjee --- zebra/zebra_rnh.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index abc2daa284..0d6728d111 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -259,18 +259,38 @@ zebra_deregister_rnh_static_nexthops (vrf_id_t vrf_id, struct nexthop *nexthop, for (nh = nexthop; nh ; nh = nh->next) { - if (nh->type == NEXTHOP_TYPE_IPV4) - { + switch (nh->type) + { + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: nh_p.family = AF_INET; nh_p.prefixlen = IPV4_MAX_BITLEN; nh_p.u.prefix4 = nh->gate.ipv4; - } - else if (nh->type == NEXTHOP_TYPE_IPV6) - { + break; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: nh_p.family = AF_INET6; nh_p.prefixlen = IPV6_MAX_BITLEN; nh_p.u.prefix6 = nh->gate.ipv6; - } + break; + /* + * Not sure what really to do here, we are not + * supposed to have either of these for NHT + * and the code has no way to know what prefix + * to use. So I'm going to just continue + * for the moment, which is preferable to + * what is currently happening which is a + * CRASH and BURN. + * Some simple testing shows that we + * are not leaving slag around for these + * skipped static routes. Since + * they don't appear to be installed + */ + case NEXTHOP_TYPE_IFINDEX: + case NEXTHOP_TYPE_BLACKHOLE: + continue; + break; + } zebra_deregister_rnh_static_nh(vrf_id, &nh_p, rn); } } -- 2.39.5