[ovs-dev] [PATCH ovn] ovn-ic: Fix route hash.

Numan Siddique numans at ovn.org
Wed Oct 21 13:37:54 UTC 2020


On Wed, Oct 14, 2020 at 10:38 PM Han Zhou <hzhou at ovn.org> 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 <i.maximets at ovn.org>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376160.html
> Fixes: 57b347c55 ("ovn-ic: Route advertisement.")
> Signed-off-by: Han Zhou <hzhou at ovn.org>

Acked-by: Numan Siddique <numans at ovn.org>

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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list