Commit 22ac8440 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[#691,!395] Added bad socket purge to IfaceMgr

src/lib/dhcp/iface_mgr.cc
    IfaceMgr::purgeBadSockets() - new function to validate
    external sockets and unregister any that are invalid.

    IfaceMgr::receive4Indirect()
    IfaceMgr::receive4Direct()
    IfaceMgr::receive6Indirect()
    IfaceMgr::receive6Direct() - added logic to all
    purgeBadSockets() when select fails with EBADF

src/lib/dhcp/tests/iface_mgr_unittest.cc
    TEST_F(IfaceMgrTest, purgeExternalSockets4)
    TEST_F(IfaceMgrTest, purgeExternalSockets6) -  new tests
    to verify bad socket purging

src/lib/http/client.cc
    Move close_callback_ reset back to Connection::close()
parent ff3d254d
......@@ -346,6 +346,23 @@ IfaceMgr::deleteExternalSocket(int socketfd) {
}
}
int
IfaceMgr::purgeBadSockets() {
std::vector<int> bad_fds;
BOOST_FOREACH(SocketCallbackInfo s, callbacks_) {
errno = 0;
if (fcntl(s.socket_, F_GETFD) < 0 && errno == EBADF) {
bad_fds.push_back(s.socket_);
}
}
for (auto bad_fd : bad_fds) {
deleteExternalSocket(bad_fd);
}
return (bad_fds.size());
}
void
IfaceMgr::deleteAllExternalSockets() {
callbacks_.clear();
......@@ -1042,6 +1059,11 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec /
// signal or for some other reason.
if (errno == EINTR) {
isc_throw(SignalInterruptOnSelect, strerror(errno));
} else if (errno == EBADF) {
int cnt = purgeBadSockets();
isc_throw(SocketReadError,
"SELECT interrupted by one invalid sockets, purged "
<< cnt << " socket descriptors");
} else {
isc_throw(SocketReadError, strerror(errno));
}
......@@ -1141,6 +1163,11 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /*
// signal or for some other reason.
if (errno == EINTR) {
isc_throw(SignalInterruptOnSelect, strerror(errno));
} else if (errno == EBADF) {
int cnt = purgeBadSockets();
isc_throw(SocketReadError,
"SELECT interrupted by one invalid sockets, purged "
<< cnt << " socket descriptors");
} else {
isc_throw(SocketReadError, strerror(errno));
}
......@@ -1265,6 +1292,11 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ )
// signal or for some other reason.
if (errno == EINTR) {
isc_throw(SignalInterruptOnSelect, strerror(errno));
} else if (errno == EBADF) {
int cnt = purgeBadSockets();
isc_throw(SocketReadError,
"SELECT interrupted by one invalid sockets, purged "
<< cnt << " socket descriptors");
} else {
isc_throw(SocketReadError, strerror(errno));
}
......@@ -1368,6 +1400,11 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
// signal or for some other reason.
if (errno == EINTR) {
isc_throw(SignalInterruptOnSelect, strerror(errno));
} else if (errno == EBADF) {
int cnt = purgeBadSockets();
isc_throw(SocketReadError,
"SELECT interrupted by one invalid sockets, purged "
<< cnt << " socket descriptors");
} else {
isc_throw(SocketReadError, strerror(errno));
}
......
......@@ -926,6 +926,16 @@ public:
/// @brief Deletes external socket
void deleteExternalSocket(int socketfd);
/// @brief Scans registered socket set and removes any that are invalid.
///
/// Walks the list of regiseterd external sockets and test each for
/// validity. If any are found to be invalid they are removed. This
/// primarily a self-defense mechanism against hook libs or other users
/// of external sockets, leaving a closed socket registered by mistake.
///
/// @return A count of the sockets purged.
int purgeBadSockets();
/// @brief Deletes all external sockets.
void deleteAllExternalSockets();
......
......@@ -665,6 +665,172 @@ public:
ASSERT_FALSE(ifacemgr->isDHCPReceiverRunning());
}
/// @brief Verifies that IfaceMgr DHCPv4 receive calls detect and
/// purge external sockets that have gone bad without affecting
/// affecting normal operations. It can be run with or without
/// packet queuing.
///
/// @param use_queue determines if packet queuing is used or not.
void purgeExternalSockets4Test(bool use_queue = false) {
bool callback_ok = false;
bool callback2_ok = false;
scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
if (use_queue) {
bool queue_enabled;
data::ConstElementPtr config = makeQueueConfig(PacketQueueMgr4::DEFAULT_QUEUE_TYPE4, 500);
ASSERT_NO_THROW(queue_enabled = ifacemgr->configureDHCPPacketQueue(AF_INET, config));
ASSERT_TRUE(queue_enabled);
// Thread should only start when there is a packet queue.
ASSERT_NO_THROW(ifacemgr->startDHCPReceiver(AF_INET));
ASSERT_TRUE(ifacemgr->isDHCPReceiverRunning());
}
// Create first pipe and register it as extra socket
int pipefd[2];
EXPECT_TRUE(pipe(pipefd) == 0);
EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0],
[this, &callback_ok](){ callback_ok = true; }));
// Let's create a second pipe and register it as well
int secondpipe[2];
EXPECT_TRUE(pipe(secondpipe) == 0);
EXPECT_NO_THROW(ifacemgr->addExternalSocket(secondpipe[0],
[this, &callback2_ok](){ callback2_ok = true; }));
// Verify a call with no data and normal external sockets works ok.
Pkt4Ptr pkt4;
ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
// No callback invocations and no DHCPv4 pkt.
EXPECT_FALSE(callback_ok);
EXPECT_FALSE(callback2_ok);
EXPECT_FALSE(pkt4);
// Now close the first pipe. This should make it's external socket invalid.
close(pipefd[1]);
close(pipefd[0]);
// We call receive4() which should detect and remove the invalid socket.
try {
pkt4 = ifacemgr->receive4(1);
ADD_FAILURE() << "receive4 should have failed";
} catch (const SocketReadError& ex) {
EXPECT_EQ(std::string("SELECT interrupted by one invalid sockets,"
" purged 1 socket descriptors"),
std::string(ex.what()));
} catch (const std::exception& ex) {
ADD_FAILURE() << "wrong exception thrown: " << ex.what();
}
// No callback invocations and no DHCPv4 pkt.
EXPECT_FALSE(callback_ok);
EXPECT_FALSE(callback2_ok);
EXPECT_FALSE(pkt4);
// Now check whether the second callback is still functional
EXPECT_EQ(38, write(secondpipe[1], "Hi, this is a message sent over a pipe", 38));
// Call recevie4 again, this should work.
ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
// Should have callback2 data only.
EXPECT_FALSE(callback_ok);
EXPECT_TRUE(callback2_ok);
EXPECT_FALSE(pkt4);
// Stop the thread. This should be no harm/no foul if we're not
// queueuing. Either way, we should not have a thread afterwards.
ASSERT_NO_THROW(ifacemgr->stopDHCPReceiver());
ASSERT_FALSE(ifacemgr->isDHCPReceiverRunning());
}
/// @brief Verifies that IfaceMgr DHCPv6 receive calls detect and
/// purge external sockets that have gone bad without affecting
/// affecting normal operations. It can be run with or without
/// packet queuing.
///
/// @param use_queue determines if packet queuing is used or not.
void purgeExternalSockets6Test(bool use_queue = false) {
bool callback_ok = false;
bool callback2_ok = false;
scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
if (use_queue) {
bool queue_enabled;
data::ConstElementPtr config = makeQueueConfig(PacketQueueMgr6::DEFAULT_QUEUE_TYPE6, 500);
ASSERT_NO_THROW(queue_enabled = ifacemgr->configureDHCPPacketQueue(AF_INET6, config));
ASSERT_TRUE(queue_enabled);
// Thread should only start when there is a packet queue.
ASSERT_NO_THROW(ifacemgr->startDHCPReceiver(AF_INET6));
ASSERT_TRUE(ifacemgr->isDHCPReceiverRunning());
}
// Create first pipe and register it as extra socket
int pipefd[2];
EXPECT_TRUE(pipe(pipefd) == 0);
EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0],
[this, &callback_ok](){ callback_ok = true; }));
// Let's create a second pipe and register it as well
int secondpipe[2];
EXPECT_TRUE(pipe(secondpipe) == 0);
EXPECT_NO_THROW(ifacemgr->addExternalSocket(secondpipe[0],
[this, &callback2_ok](){ callback2_ok = true; }));
// Verify a call with no data and normal external sockets works ok.
Pkt6Ptr pkt6;
ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
// No callback invocations and no DHCPv6 pkt.
EXPECT_FALSE(callback_ok);
EXPECT_FALSE(callback2_ok);
EXPECT_FALSE(pkt6);
// Now close the first pipe. This should make it's external socket invalid.
close(pipefd[1]);
close(pipefd[0]);
// We call receive6() which should detect and remove the invalid socket.
try {
pkt6 = ifacemgr->receive6(1);
ADD_FAILURE() << "receive6 should have failed";
} catch (const SocketReadError& ex) {
EXPECT_EQ(std::string("SELECT interrupted by one invalid sockets,"
" purged 1 socket descriptors"),
std::string(ex.what()));
} catch (const std::exception& ex) {
ADD_FAILURE() << "wrong exception thrown: " << ex.what();
}
// No callback invocations and no DHCPv6 pkt.
EXPECT_FALSE(callback_ok);
EXPECT_FALSE(callback2_ok);
EXPECT_FALSE(pkt6);
// Now check whether the second callback is still functional
EXPECT_EQ(38, write(secondpipe[1], "Hi, this is a message sent over a pipe", 38));
// Call recevie6 again, this should work.
ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
// Should have callback2 data only.
EXPECT_FALSE(callback_ok);
EXPECT_TRUE(callback2_ok);
EXPECT_FALSE(pkt6);
// Stop the thread. This should be no harm/no foul if we're not
// queueuing. Either way, we should not have a thread afterwards.
ASSERT_NO_THROW(ifacemgr->stopDHCPReceiver());
ASSERT_FALSE(ifacemgr->isDHCPReceiverRunning());
}
/// Holds the invocation counter for ifaceMgrErrorHandler.
int errors_count_;
......@@ -2760,6 +2926,18 @@ TEST_F(IfaceMgrTest, DeleteExternalSockets4) {
close(secondpipe[0]);
}
// Tests that an existing external socket that becomes invalid
// is detected and purged, without affecting other sockets.
// Tests uses receive4().
TEST_F(IfaceMgrTest, purgeExternalSockets4) {
// Run purge test without packet queuing.
purgeExternalSockets4Test();
// Run purge test with packet queuing.
purgeExternalSockets4Test(true);
}
// Tests if a single external socket and its callback can be passed and
// it is supported properly by receive6() method.
......@@ -2929,6 +3107,15 @@ TEST_F(IfaceMgrTest, DeleteExternalSockets6) {
close(secondpipe[0]);
}
// Tests if existing external socket become invalid and be purged and not
// not affect any other existing sockets. Tests uses receive6()
TEST_F(IfaceMgrTest, purgeExternalSockets6) {
// Run purge test without packet queuing.
purgeExternalSockets6Test();
// Run purge test with packet queuing.
purgeExternalSockets6Test(true);
}
// Test checks if the unicast sockets can be opened.
// This test is now disabled, because there is no reliable way to test it. We
......
......@@ -500,7 +500,6 @@ Connection::resetState() {
current_response_.reset();
parser_.reset();
current_callback_ = HttpClient::RequestHandler();
close_callback_ = HttpClient::CloseHandler();
}
void
......@@ -571,6 +570,7 @@ void
Connection::close() {
if (close_callback_) {
close_callback_(socket_.getNative());
close_callback_ = HttpClient::CloseHandler();
}
timer_.cancel();
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment