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

Han Zhou hzhou at ovn.org
Wed Oct 21 16:13:48 UTC 2020


On Wed, Oct 21, 2020 at 6:38 AM Numan Siddique <numans at ovn.org> wrote:
>
> 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

Thanks Numan. I applied this to master and backported up to branch 20.03.

Han


More information about the dev mailing list