From 268b533be7e151239c577934083b70c506500c46 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 27 Oct 2020 09:59:10 -0400 Subject: [PATCH] isisd: Fix usage of uninited memory valgrind is showing a usage of uninited memory: ==935465== Conditional jump or move depends on uninitialised value(s) ==935465== at 0x159E17: tlvs_area_addresses_to_adj (isis_tlvs.c:4430) ==935465== by 0x15A4BD: isis_tlvs_to_adj (isis_tlvs.c:4568) ==935465== by 0x1377F0: process_p2p_hello (isis_pdu.c:203) ==935465== by 0x1391FD: process_hello (isis_pdu.c:781) ==935465== by 0x13BDBE: isis_handle_pdu (isis_pdu.c:1700) ==935465== by 0x13BECD: isis_receive (isis_pdu.c:1744) ==935465== by 0x49210FF: thread_call (thread.c:1585) ==935465== by 0x48CFACB: frr_run (libfrr.c:1099) ==935465== by 0x1218C9: main (isis_main.c:272) ==935465== ==935465== Conditional jump or move depends on uninitialised value(s) ==935465== at 0x483EEC5: bcmp (vg_replace_strmem.c:1111) ==935465== by 0x15A290: tlvs_ipv4_addresses_to_adj (isis_tlvs.c:4512) ==935465== by 0x15A4EB: isis_tlvs_to_adj (isis_tlvs.c:4570) ==935465== by 0x1377F0: process_p2p_hello (isis_pdu.c:203) ==935465== by 0x1391FD: process_hello (isis_pdu.c:781) ==935465== by 0x13BDBE: isis_handle_pdu (isis_pdu.c:1700) ==935465== by 0x13BECD: isis_receive (isis_pdu.c:1744) ==935465== by 0x49210FF: thread_call (thread.c:1585) ==935465== by 0x48CFACB: frr_run (libfrr.c:1099) ==935465== by 0x1218C9: main (isis_main.c:272) Effectively we are reallocing memory to hold data. realloc does not set the new memory to anything. So whatever happens to be in the memory is what is there. after the realloc happens we are iterating over the memory just realloced and doing memcmp's to values in it causing these use of uninitialized memory. Signed-off-by: Donald Sharp --- isisd/isis_tlvs.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c index ea429561e2..75bd62620b 100644 --- a/isisd/isis_tlvs.c +++ b/isisd/isis_tlvs.c @@ -4414,11 +4414,19 @@ static void tlvs_area_addresses_to_adj(struct isis_tlvs *tlvs, bool *changed) { if (adj->area_address_count != tlvs->area_addresses.count) { + uint32_t oc = adj->area_address_count; + *changed = true; adj->area_address_count = tlvs->area_addresses.count; adj->area_addresses = XREALLOC( MTYPE_ISIS_ADJACENCY_INFO, adj->area_addresses, adj->area_address_count * sizeof(*adj->area_addresses)); + + for (; oc < adj->area_address_count; oc++) { + adj->area_addresses[oc].addr_len = 0; + memset(&adj->area_addresses[oc].area_addr, 0, + sizeof(adj->area_addresses[oc].area_addr)); + } } struct isis_area_address *addr = NULL; @@ -4496,11 +4504,18 @@ static void tlvs_ipv4_addresses_to_adj(struct isis_tlvs *tlvs, hook_call(isis_adj_ip_disabled_hook, adj, AF_INET); if (adj->ipv4_address_count != tlvs->ipv4_address.count) { + uint32_t oc = adj->ipv4_address_count; + *changed = true; adj->ipv4_address_count = tlvs->ipv4_address.count; adj->ipv4_addresses = XREALLOC( MTYPE_ISIS_ADJACENCY_INFO, adj->ipv4_addresses, adj->ipv4_address_count * sizeof(*adj->ipv4_addresses)); + + for (; oc < adj->ipv4_address_count; oc++) { + memset(&adj->ipv4_addresses[oc], 0, + sizeof(adj->ipv4_addresses[oc])); + } } struct isis_ipv4_address *addr = NULL; @@ -4535,11 +4550,18 @@ static void tlvs_ipv6_addresses_to_adj(struct isis_tlvs *tlvs, hook_call(isis_adj_ip_disabled_hook, adj, AF_INET6); if (adj->ipv6_address_count != tlvs->ipv6_address.count) { + uint32_t oc = adj->ipv6_address_count; + *changed = true; adj->ipv6_address_count = tlvs->ipv6_address.count; adj->ipv6_addresses = XREALLOC( MTYPE_ISIS_ADJACENCY_INFO, adj->ipv6_addresses, adj->ipv6_address_count * sizeof(*adj->ipv6_addresses)); + + for (; oc < adj->ipv6_address_count; oc++) { + memset(&adj->ipv6_addresses[oc], 0, + sizeof(adj->ipv6_addresses[oc])); + } } struct isis_ipv6_address *addr = NULL; -- 2.39.5