Fix ifaddrs error handling.

An NLMSG_ERROR packet includes an errno value that we should use. Also report
failures to create a socket immediately, rather than falling through to the
send and reporting EBADF.

Bug: http://b/32145516
Bug: http://b/31038971
Test: bionic ifaddr tests on ryu (with broken kernel) and flounder
Change-Id: I84c480c5b75077eb90d40426a9d66d7bffbd3d51
This commit is contained in:
Elliott Hughes 2016-10-14 12:15:32 -07:00
parent 49ef8c822f
commit 22950687ee
2 changed files with 45 additions and 1 deletions

View file

@ -61,6 +61,7 @@ bool NetlinkConnection::SendRequest(int type) {
// Did we open a netlink socket yet?
if (fd_ == -1) {
fd_ = socket(PF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, NETLINK_ROUTE);
if (fd_ == -1) return false;
}
// Construct and send the message.
@ -83,7 +84,11 @@ bool NetlinkConnection::ReadResponses(void callback(void*, nlmsghdr*), void* con
nlmsghdr* hdr = reinterpret_cast<nlmsghdr*>(data_);
for (; NLMSG_OK(hdr, static_cast<size_t>(bytes_read)); hdr = NLMSG_NEXT(hdr, bytes_read)) {
if (hdr->nlmsg_type == NLMSG_DONE) return true;
if (hdr->nlmsg_type == NLMSG_ERROR) return false;
if (hdr->nlmsg_type == NLMSG_ERROR) {
nlmsgerr* err = reinterpret_cast<nlmsgerr*>(NLMSG_DATA(hdr));
errno = (hdr->nlmsg_len >= NLMSG_LENGTH(sizeof(nlmsgerr))) ? -err->error : EIO;
return false;
}
callback(context, hdr);
}
}

View file

@ -19,6 +19,7 @@
#include <ifaddrs.h>
#include <dirent.h>
#include <fcntl.h>
#include <linux/if_packet.h>
#include <net/ethernet.h>
#include <net/if.h>
@ -28,6 +29,7 @@
#include <algorithm>
#include <map>
#include <thread>
#include <vector>
TEST(ifaddrs, freeifaddrs_null) {
@ -267,3 +269,40 @@ TEST(ifaddrs, inet6_scope_ids) {
freeifaddrs(addrs);
}
TEST(ifaddrs, kernel_bug_31038971) {
// Some kernels had a bug that would lead to an NLMSG_ERROR response,
// but bionic wasn't setting errno based on the value in the message.
// This is the test for the kernel bug, but on a device with a bad
// kernel this test was also useful for testing the bionic errno fix.
std::vector<std::thread*> threads;
for (size_t i = 0; i < 128; ++i) {
threads.push_back(new std::thread([]() {
ifaddrs* addrs = nullptr;
ASSERT_EQ(0, getifaddrs(&addrs)) << strerror(errno);
freeifaddrs(addrs);
}));
}
for (auto& t : threads) {
t->join();
delete t;
}
}
TEST(ifaddrs, errno_EMFILE) {
std::vector<int> fds;
while (true) {
int fd = open("/dev/null", O_RDONLY|O_CLOEXEC);
if (fd == -1) {
ASSERT_EQ(EMFILE, errno);
break;
}
fds.push_back(fd);
}
ifaddrs* addrs;
EXPECT_EQ(-1, getifaddrs(&addrs));
EXPECT_EQ(EMFILE, errno);
for (int fd : fds) close(fd);
}