BIND 10 trac2765, updated. 183693547ce28671d56bc272859b401c47db8f33 [2765] Close fallback sockets together with primary sockets.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Nov 26 18:12:04 UTC 2013
The branch, trac2765 has been updated
via 183693547ce28671d56bc272859b401c47db8f33 (commit)
via e4d1f908e14c82062aab9a391299eb0b2692bf9a (commit)
from d0506dcfcf7798859931851c3eb90956dd4bb03c (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 183693547ce28671d56bc272859b401c47db8f33
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue Nov 26 19:10:35 2013 +0100
[2765] Close fallback sockets together with primary sockets.
commit e4d1f908e14c82062aab9a391299eb0b2692bf9a
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue Nov 26 19:00:47 2013 +0100
[2765] Open fallback socket when LPF is used.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/iface_mgr.cc | 8 ++++++++
src/lib/dhcp/pkt_filter_lpf.cc | 29 ++++++++++-------------------
src/lib/dhcp/tests/iface_mgr_unittest.cc | 30 +++++++++++++++++++++---------
3 files changed, 39 insertions(+), 28 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index 3b33671..845fd21 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -88,6 +88,10 @@ Iface::closeSockets(const uint16_t family) {
// Close and delete the socket and move to the
// next one.
close(sock->sockfd_);
+ // Close fallback socket if open.
+ if (sock->fallbackfd_) {
+ close(sock->fallbackfd_);
+ }
sockets_.erase(sock++);
} else {
@@ -148,6 +152,10 @@ bool Iface::delSocket(uint16_t sockfd) {
while (sock!=sockets_.end()) {
if (sock->sockfd_ == sockfd) {
close(sockfd);
+ // Close fallback socket if open.
+ if (sock->fallbackfd_) {
+ close(sock->fallbackfd_);
+ }
sockets_.erase(sock);
return (true); //socket found
}
diff --git a/src/lib/dhcp/pkt_filter_lpf.cc b/src/lib/dhcp/pkt_filter_lpf.cc
index 81d53f2..7bb4dac 100644
--- a/src/lib/dhcp/pkt_filter_lpf.cc
+++ b/src/lib/dhcp/pkt_filter_lpf.cc
@@ -107,28 +107,17 @@ PktFilterLPF::openSocket(const Iface& iface,
const isc::asiolink::IOAddress& addr,
const uint16_t port, const bool,
const bool) {
- // Let's check if a socket is already in use
- int sock_check = socket(AF_INET, SOCK_DGRAM, 0);
- if (sock_check < 0) {
- isc_throw(SocketConfigError, "Failed to create dgram socket");
- }
- struct sockaddr_in addr4;
- memset(& addr4, 0, sizeof(addr4));
- addr4.sin_family = AF_INET;
- addr4.sin_addr.s_addr = htonl(addr);
- addr4.sin_port = htons(port);
-
- if (bind(sock_check, (struct sockaddr *)& addr4, sizeof(addr4)) < 0) {
- // We return negative, the proper error message will be displayed
- // by the IfaceMgr ...
- close(sock_check);
- return (SocketInfo(addr, port, -1));
- }
- close(sock_check);
+ // Open fallback socket first. If it fails, it will give us an indication
+ // that there is another service (perhaps DHCP server) running.
+ // The function will throw an exception and effectivelly cease opening
+ // raw socket below.
+ int fallback = openFallbackSocket(addr, port);
+ // The fallback is open, so we are good to open primary socket.
int sock = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
if (sock < 0) {
+ close(fallback);
isc_throw(SocketConfigError, "Failed to create raw LPF socket");
}
@@ -146,6 +135,7 @@ PktFilterLPF::openSocket(const Iface& iface,
if (setsockopt(sock, SOL_SOCKET, SO_ATTACH_FILTER, &filter_program,
sizeof(filter_program)) < 0) {
close(sock);
+ close(fallback);
isc_throw(SocketConfigError, "Failed to install packet filtering program"
<< " on the socket " << sock);
}
@@ -162,11 +152,12 @@ PktFilterLPF::openSocket(const Iface& iface,
if (bind(sock, reinterpret_cast<const struct sockaddr*>(&sa),
sizeof(sa)) < 0) {
close(sock);
+ close(fallback);
isc_throw(SocketConfigError, "Failed to bind LPF socket '" << sock
<< "' to interface '" << iface.getName() << "'");
}
- SocketInfo sock_desc(addr, port, sock);
+ SocketInfo sock_desc(addr, port, sock, fallback);
return (sock_desc);
}
diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index f2bb773..3d2ed01 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -117,7 +117,13 @@ public:
IfaceCollection & getIfacesLst() { return ifaces_; }
};
-// Dummy class for now, but this will be expanded when needed
+/// @brief A test fixture class for IfaceMgr.
+///
+/// @todo Sockets being opened by IfaceMgr tests should be managed by
+/// the test fixture. In particular, the class should close sockets after
+/// each test. Current approach where test cases are responsible for
+/// closing sockets is resource leak prone, especially in case of the
+/// test failure path.
class IfaceMgrTest : public ::testing::Test {
public:
// These are empty for now, but let's keep them around
@@ -1007,17 +1013,23 @@ TEST_F(IfaceMgrTest, checkPacketFilterLPFSocket) {
// Then the second use PkFilterLPF mode
EXPECT_NO_THROW(iface_mgr2->setMatchingPacketFilter(true));
- // This socket opening attempt should not return positive value
- // The first socket already opened same port
- EXPECT_NO_THROW(
+
+ // The socket is open and bound. Another attempt to open socket and
+ // bind to the same address and port should result in an exception.
+ EXPECT_THROW(
socket2 = iface_mgr2->openSocket(LOOPBACK, loAddr,
- DHCP4_SERVER_PORT + 10000);
+ DHCP4_SERVER_PORT + 10000),
+ isc::dhcp::SocketConfigError
);
+ // Surprisingly we managed to open another socket. We have to close it
+ // to prevent resource leak.
+ if (socket2 >= 0) {
+ close(socket2);
+ }
- EXPECT_LE(socket2, 0);
-
- close(socket2);
- close(socket1);
+ if (socket1 >= 0) {
+ close(socket1);
+ }
}
#else
More information about the bind10-changes
mailing list