From bdd4ddd8fae7ba14548e1b6f01f7b9d143261db2 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 2 Dec 2015 11:08:56 -0800 Subject: QNetworkInterface: fix support for address labels on Linux interfaces Commit 64a1448d87727878d9789906b2f4f5b9e3d74e38 (Qt 5.2) caused QNetworkInterface to report address labels (a.k.a. interface aliases) as separate interfaces. This is caused by the fact that glibc, uClibc and MUSL copy the address label (netlink address attribute IFA_LABEL) to the ifa_name field, which made QNetworkInterfaceManager think that it was an interface it hadn't yet seen. Address labels are the old way to add more than one IP address to an interface on Linux, for example: ifconfig eth0:1 192.0.2.2 Those do not create a new interface, so the "eth0:1" label maps to the same interface index as the parent interface. This has been deprecated for 10 years, but there are still tools out there that add addresses in this manner. This commit restores behavior compatibility with Qt 4.2-5.1. The Qt 5.2-5.5 behavior is incorrect because it reports more than one interface with the same index. On systems configured like the above, the tst_QNetworkInterface::interfaceFromXXX test was failing. Change-Id: I8de47ed6c7be4847b99bffff141c2d9de8cf7329 Reviewed-by: Richard J. Moore --- src/network/kernel/qnetworkinterface_unix.cpp | 38 ++++++++++++++++++---- .../qnetworkinterface/tst_qnetworkinterface.cpp | 19 +++++++++++ 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/network/kernel/qnetworkinterface_unix.cpp b/src/network/kernel/qnetworkinterface_unix.cpp index cc53087024..5e6d24dd44 100644 --- a/src/network/kernel/qnetworkinterface_unix.cpp +++ b/src/network/kernel/qnetworkinterface_unix.cpp @@ -314,10 +314,15 @@ static QList createInterfaces(ifaddrs *rawList) { QList interfaces; QSet seenInterfaces; - - // on Linux, AF_PACKET addresses carry the hardware address and interface index; - // scan for them first (they're usually first, but we have no guarantee this - // will be the case forever) + QVarLengthArray seenIndexes; // faster than QSet + + // On Linux, glibc, uClibc and MUSL obtain the address listing via two + // netlink calls: first an RTM_GETLINK to obtain the interface listing, + // then one RTM_GETADDR to get all the addresses (uClibc implementation is + // copied from glibc; Bionic currently doesn't support getifaddrs). They + // synthesize AF_PACKET addresses from the RTM_GETLINK responses, which + // means by construction they currently show up first in the interface + // listing. for (ifaddrs *ptr = rawList; ptr; ptr = ptr->ifa_next) { if (ptr->ifa_addr && ptr->ifa_addr->sa_family == AF_PACKET) { sockaddr_ll *sll = (sockaddr_ll *)ptr->ifa_addr; @@ -328,23 +333,30 @@ static QList createInterfaces(ifaddrs *rawList) iface->flags = convertFlags(ptr->ifa_flags); iface->hardwareAddress = iface->makeHwAddress(sll->sll_halen, (uchar*)sll->sll_addr); + Q_ASSERT(!seenIndexes.contains(iface->index)); + seenIndexes.append(iface->index); seenInterfaces.insert(iface->name); } } // see if we missed anything: - // virtual interfaces with no HW address have no AF_PACKET + // - virtual interfaces with no HW address have no AF_PACKET + // - interface labels have no AF_PACKET, but shouldn't be shown as a new interface for (ifaddrs *ptr = rawList; ptr; ptr = ptr->ifa_next) { if (ptr->ifa_addr && ptr->ifa_addr->sa_family != AF_PACKET) { QString name = QString::fromLatin1(ptr->ifa_name); if (seenInterfaces.contains(name)) continue; + int ifindex = if_nametoindex(ptr->ifa_name); + if (seenIndexes.contains(ifindex)) + continue; + QNetworkInterfacePrivate *iface = new QNetworkInterfacePrivate; interfaces << iface; iface->name = name; iface->flags = convertFlags(ptr->ifa_flags); - iface->index = if_nametoindex(ptr->ifa_name); + iface->index = ifindex; } } @@ -423,7 +435,7 @@ static QList interfaceListing() interfaces = createInterfaces(interfaceListing); for (ifaddrs *ptr = interfaceListing; ptr; ptr = ptr->ifa_next) { - // Get the interface index + // Find the interface QString name = QString::fromLatin1(ptr->ifa_name); QNetworkInterfacePrivate *iface = 0; QList::Iterator if_it = interfaces.begin(); @@ -433,6 +445,18 @@ static QList interfaceListing() iface = *if_it; break; } + + if (!iface) { + // it may be an interface label, search by interface index + int ifindex = if_nametoindex(ptr->ifa_name); + for (if_it = interfaces.begin(); if_it != interfaces.end(); ++if_it) + if ((*if_it)->index == ifindex) { + // found this interface already + iface = *if_it; + break; + } + } + if (!iface) { // skip all non-IP interfaces continue; diff --git a/tests/auto/network/kernel/qnetworkinterface/tst_qnetworkinterface.cpp b/tests/auto/network/kernel/qnetworkinterface/tst_qnetworkinterface.cpp index f7798bbb70..519ee0dc84 100644 --- a/tests/auto/network/kernel/qnetworkinterface/tst_qnetworkinterface.cpp +++ b/tests/auto/network/kernel/qnetworkinterface/tst_qnetworkinterface.cpp @@ -55,6 +55,7 @@ private slots: void initTestCase(); void cleanupTestCase(); void dump(); + void consistencyCheck(); void loopbackIPv4(); void loopbackIPv6(); void localAddress(); @@ -148,6 +149,24 @@ void tst_QNetworkInterface::dump() } } +void tst_QNetworkInterface::consistencyCheck() +{ + QList ifaces = QNetworkInterface::allInterfaces(); + QSet interfaceNames; + QVector interfaceIndexes; + + foreach (const QNetworkInterface &iface, ifaces) { + QVERIFY2(!interfaceNames.contains(iface.name()), + "duplicate name = " + iface.name().toLocal8Bit()); + interfaceNames << iface.name(); + + QVERIFY2(!interfaceIndexes.contains(iface.index()), + "duplicate index = " + QByteArray::number(iface.index())); + if (iface.index()) + interfaceIndexes << iface.index(); + } +} + void tst_QNetworkInterface::loopbackIPv4() { QList all = QNetworkInterface::allAddresses(); -- cgit v1.2.3