> From: "Daniel Alvarez" <[email protected]> > To: [email protected] > Sent: Friday, 21 July, 2017 11:28:24 AM > Subject: [ovs-dev] [PATCH v3] netdev: check for NULL fields in > netdev_get_addrs > > When the interfaces list is retrieved through getiffaddrs(), there > might be elements with iface_name set to NULL. > > This patch checks ifa_name to be not NULL before comparing it to the > actual device name in the loop that calculates how many interfaces > exist with that same name. > > Also, this patch checks that ifa_netmask is not NULL for coherence > with the existing code so that it doesn't allocate more memory > than needed if this field is NULL. > > Note, that these checks are already being done later in the function > so it should be done in both places. > > Signed-off-by: Daniel Alvarez <[email protected]> > --- > v2 -> v3: fix email formatting since v2 wasn't correctly picked by > patchwork. > > I've been debugging a coredump produced by a segmentation fault of > ovs-vswitchd. It seems to be caused by a NULL pointer passed to > strncmp by netdev_get_addrs() function: > > #0 0x00007fd840e2d34c in ?? () from /lib64/libc.so.6 > #1 0x00007fd842ae63b6 in netdev_get_addrs (dev=0x7fd844e1e750 "vlan121", > paddr=paddr@entry=0x7ffe833244a0, pmask=pmask@entry=0x7ffe83324498, > n_in=n_in@entry=0x7ffe83324494) > at lib/netdev.c:1890 > #2 0x00007fd842b70365 in netdev_linux_get_addr_list (netdev_=0x7fd844e8ec40, > addr=0x7ffe833244a0, mask=0x7ffe83324498, n_cnt=0x7ffe83324494) at > lib/netdev-linux.c:2517 > #3 0x00007fd842ae576f in netdev_get_addr_list (netdev=<optimized out>, > addr=addr@entry=0x7ffe833244a0, mask=mask@entry=0x7ffe83324498, > n_addr=n_addr@entry=0x7ffe83324494) > at lib/netdev.c:1133 > #4 0x00007fd842b30191 in get_src_addr (ip6_dst=ip6_dst@entry=0x7ffe8332522c, > output_bridge=output_bridge@entry=0x7ffe8332524c "vlan121", > psrc=psrc@entry=0x7fd844e6f0a0) > at lib/ovs-router.c:146 > #5 0x00007fd842b30655 in ovs_router_insert__ (priority=<optimized out>, > ip6_dst=ip6_dst@entry=0x7ffe8332522c, plen=<optimized out>, > output_bridge=output_bridge@entry=0x7ffe8332524c "vlan121", > gw=gw@entry=0x7ffe8332523c) at lib/ovs-router.c:200 > #6 0x00007fd842b30e37 in ovs_router_insert > (ip_dst=ip_dst@entry=0x7ffe8332522c, plen=<optimized out>, > output_bridge=output_bridge@entry=0x7ffe8332524c "vlan121", > gw=gw@entry=0x7ffe8332523c) at lib/ovs-router.c:228 > #7 0x00007fd842b79d24 in route_table_handle_msg (change=0x7ffe83325220) at > lib/route-table.c:295 > #8 route_table_reset () at lib/route-table.c:174 > #9 0x00007fd842b79ef5 in route_table_run () at lib/route-table.c:127 > #10 0x00007fd842ae3701 in netdev_vport_run (netdev_class=<optimized out>) at > lib/netdev-vport.c:319 > #11 0x00007fd842ae438e in netdev_run () at lib/netdev.c:163 > #12 0x00007fd8428f329c in main (argc=10, argv=0x7ffe833265a8) at > vswitchd/ovs-vswitchd.c:114 > > In frame 1 we can confirm this: > > (gdb) p *ifa > $94 = {ifa_next = 0x7fd8451c2a78, ifa_name = 0x0, ifa_flags = 0, ifa_addr = > 0x7fd8451c29f8, ifa_netmask = 0x7fd8451c2a1c, ifa_ifu = {ifu_broadaddr = > 0x0, ifu_dstaddr = 0x0}, ifa_data = 0x0} > > (gdb) list > 1885 if (ifa->ifa_addr != NULL) { > 1886 int family; > 1887 > 1888 family = ifa->ifa_addr->sa_family; > 1889 if (family == AF_INET || family == AF_INET6) { > 1890 if (!strncmp(ifa->ifa_name, dev, IFNAMSIZ)) { > 1891 cnt++; > 1892 } > 1893 } > 1894 } > > > Later in the code, we're checking for ifa_name [0] not NULL so it > makes sense to check it as well where this patch does it. > > Also, as it's not expected to get an unnamed interface, it > may happen and also iproute2 checks this condition when retrieving > the interfaces list via netlink [1]. > > [0] https://github.com/openvswitch/ovs/blob/master/lib/netdev.c#L1970 > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git/tree/ip/ipaddress.c#n664 > > lib/netdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/netdev.c b/lib/netdev.c > index 0d5fad5..eed4d09 100644 > --- a/lib/netdev.c > +++ b/lib/netdev.c > @@ -1946,7 +1946,7 @@ netdev_get_addrs(const char dev[], struct in6_addr > **paddr, > } > > for (ifa = if_addr_list; ifa; ifa = ifa->ifa_next) { > - if (ifa->ifa_addr != NULL) { > + if (ifa->ifa_addr && ifa->ifa_name && ifa->ifa_netmask) { > int family; > > family = ifa->ifa_addr->sa_family; > -- > 1.8.3.1 >
Makes sense, LGTM. Acked-by: Lance Richardson <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
