On Wed, Oct 14, 2020 at 10:38 PM Han Zhou <[email protected]> wrote: > > The 'nexthop' that passed to ic_route_hash() is not fully initialized in > get_nexthop_from_lport_addresses(). 'nexthop' has type of 'struct v46_ip' > which > contains a union to share space for ipv4 and ipv6 address. If only ipv4 > initialized where is a plenty of uninitialized space that goes to > hash_bytes(nexthop, sizeof *nexthop, basis). > > Impact: there are two places where this function is called. > > 1. In add_to_routes_ad(), the nexthop is initialized in parse_route() before > calling get_nexthop_from_lport_addresses(), luckily. > > 2. In add_network_to_routes_ad(), we are unlucky. When a directly connected > network of a router is found to be advertised, if the route already existed in > the global IC-SB, it may not be found due to the hash difference, and results > in the existing route being deleted and the same one recreated, unnecessarily. > > This patch fixes the problem by initializing the struct to zero before setting > the fields. > > From Ilya's report: > > Report from MemorySanitizer: > > > > ==3074629==WARNING: MemorySanitizer: use-of-uninitialized-value > > #0 0x67177e in mhash_add__ ovs/./lib/hash.h:66:9 > > #1 0x671668 in mhash_add ovs/./lib/hash.h:78:12 > > #2 0x6701e9 in hash_bytes ovs/lib/hash.c:38:16 > > #3 0x524b4a in add_network_to_routes_ad ic/ovn-ic.c:1095:5 > > #4 0x51eea3 in route_run ic/ovn-ic.c:1424:21 > > #5 0x51887b in main ic/ovn-ic.c:1674:17 > > #6 0x7fd4ce7871a2 in __libc_start_main > > #7 0x49c90d in _start (ic/ovn-ic+0x49c90d) > > > > Uninitialized value was created by an allocation of 'nexthop' in the > > stack frame of function 'add_network_to_routes_ad' > > #0 0x5245f0 in add_network_to_routes_ad ic/ovn-ic.c:1069 > > Reported-by: Ilya Maximets <[email protected]> > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376160.html > Fixes: 57b347c55 ("ovn-ic: Route advertisement.") > Signed-off-by: Han Zhou <[email protected]>
Acked-by: Numan Siddique <[email protected]> Thanks Numan > --- > ic/ovn-ic.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index dc9bcc64e..923969fff 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -907,6 +907,7 @@ get_nexthop_from_lport_addresses(int family, > const struct lport_addresses *laddr, > struct v46_ip *nexthop) > { > + memset(nexthop, 0, sizeof *nexthop); > nexthop->family = family; > if (family == AF_INET) { > if (!laddr->n_ipv4_addrs) { > -- > 2.26.2 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
