[master] fixed Iface read_buffer_ double free (#3712)

diff --git a/ChangeLog b/ChangeLog
index a78d2cd1267baabb7c2b750eef44bd52f5533bb2..6ac6de5e0dc2d0344a3153b9745abc9abfdb5597 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+897.   [bug]           fdupont
+       Removed a double free of the read_buffer_ field of Iface
+       objects after (spurious) copy (partial as copies are not
+       yet fixed).
+       (Trac #3712, git )
+
 896.   [bug]           fdupont
        Removed exit() in D2 for version command line processing.
        This interfered with how the unit-tests were run.

diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h
index 0e34c0506541d094e8ee46b36ec17774f8d1ade2..1227a7c78c7e2ff1a767b0f77a7cfbce7932c815 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.h
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -71,7 +71,7 @@ public:
     }
 
     /// Does nothing.
-    virtual Pkt4Ptr receive(const Iface&, const SocketInfo&) {
+    virtual Pkt4Ptr receive(Iface&, const SocketInfo&) {
         return Pkt4Ptr();
     }
 

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index c34fa7bbccefddeb3422265db1842dc1676ee8ec..d016e9f199c15ff5214f187d0144fca8ddf45301 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -56,18 +56,11 @@ Iface::Iface(const std::string& name, int ifindex)
     :name_(name), ifindex_(ifindex), mac_len_(0), hardware_type_(0),
      flag_loopback_(false), flag_up_(false), flag_running_(false),
      flag_multicast_(false), flag_broadcast_(false), flags_(0),
-     inactive4_(false), inactive6_(false), read_buffer_(NULL),
-     read_buffer_size_(0)
+     inactive4_(false), inactive6_(false)
 {
     memset(mac_, 0, sizeof(mac_));
 }
 
-Iface::~Iface() {
-    if (read_buffer_ != NULL) {
-        free(read_buffer_);
-    }
-}
-
 void
 Iface::closeSockets() {
     // Close IPv4 sockets.
@@ -175,24 +168,6 @@ bool Iface::delSocket(const uint16_t sockfd) {
     return (false); // socket not found
 }
 
-void
-Iface::resizeReadBuffer(const size_t new_size) {
-    // Do nothing if the new size is equal to the current size.
-    if (new_size == read_buffer_size_) {
-        return;
-    }
-
-    read_buffer_size_ = new_size;
-    read_buffer_ = static_cast<uint8_t*>(realloc(read_buffer_,
-                                                 read_buffer_size_));
-    if (read_buffer_ == NULL) {
-        free(read_buffer_);
-        read_buffer_size_ = 0;
-        isc_throw(SocketConfigError, "failed to resize the socket read"
-                  " buffer");
-    }
-}
-
 IfaceMgr::IfaceMgr()
     :control_buf_len_(CMSG_SPACE(sizeof(struct in6_pktinfo))),
      control_buf_(new char[control_buf_len_]),
@@ -943,7 +918,7 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
                   " one million microseconds");
     }
     const SocketInfo* candidate = 0;
-    IfaceCollection::const_iterator iface;
+    IfaceCollection::iterator iface;
     fd_set sockets;
     int maxfd = 0;
 

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index a04509c35680a5b563c0e754a5e1f2072957828e..700487fca5dcf9eb04db6f0c7076f1440ea02913 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -30,6 +30,7 @@
 #include <boost/shared_ptr.hpp>
 
 #include <list>
+#include <vector>
 
 namespace isc {
 
@@ -174,9 +175,7 @@ public:
     Iface(const std::string& name, int ifindex);
 
     /// @brief Destructor.
-    ///
-    /// Deallocates the socket read buffer.
-    ~Iface();
+    ~Iface() { }
 
     /// @brief Closes all open sockets on interface.
     void closeSockets();
@@ -398,19 +397,24 @@ public:
     ///
     /// @return Pointer to the first element of the read buffer or
     /// NULL if the buffer is empty.
-    uint8_t* getReadBuffer() const {
-        return (read_buffer_);
+    uint8_t* getReadBuffer() {
+        if (read_buffer_.empty()) {
+            return NULL;
+        }
+        return (&read_buffer_[0]);
     }
 
     /// @brief Returns the current size of the socket read buffer.
     size_t getReadBufferSize() const {
-        return (read_buffer_size_);
+        return (read_buffer_.size());
     }
 
     /// @brief Reallocates the socket read buffer.
     ///
     /// @param new_size New size of the buffer.
-    void resizeReadBuffer(const size_t new_size);
+    void resizeReadBuffer(const size_t new_size) {
+        read_buffer_.resize(new_size);
+    }
 
 protected:
     /// Socket used to send data.
@@ -473,13 +477,10 @@ public:
 
 private:
 
-    /// @brief Pointer to the buffer holding the data read from the socket.
+    /// @brief The buffer holding the data read from the socket.
     ///
     /// See @c Iface manager description for details.
-    uint8_t* read_buffer_;
-
-    /// @brief Allocated size of the read buffer.
-    size_t read_buffer_size_;
+    std::vector<uint8_t> read_buffer_;
 };
 
 /// @brief This type describes the callback function invoked when error occurs

diff --git a/src/lib/dhcp/pkt_filter.h b/src/lib/dhcp/pkt_filter.h
index c58cc09249aa3f8c3960d639417744aa2615f969..dc1e96796d2de9c3e6fb2c8cd7b4173417a2ec3b 100644 (file)
--- a/src/lib/dhcp/pkt_filter.h
+++ b/src/lib/dhcp/pkt_filter.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -98,7 +98,7 @@ public:
     /// @param socket_info structure holding socket information
     ///
     /// @return Received packet
-    virtual Pkt4Ptr receive(const Iface& iface,
+    virtual Pkt4Ptr receive(Iface& iface,
                             const SocketInfo& socket_info) = 0;
 
     /// @brief Send packet over specified socket.

diff --git a/src/lib/dhcp/pkt_filter_bpf.cc b/src/lib/dhcp/pkt_filter_bpf.cc
index 36262be125763e02412fc77c6b389cfcdd77c282..9db4cb17c8f3eba0f5afa1e0c48bc74e491bf26b 100644 (file)
--- a/src/lib/dhcp/pkt_filter_bpf.cc
+++ b/src/lib/dhcp/pkt_filter_bpf.cc
@@ -379,11 +379,11 @@ PktFilterBPF::openSocket(Iface& iface,
 }
 
 Pkt4Ptr
-PktFilterBPF::receive(const Iface& iface, const SocketInfo& socket_info) {
+PktFilterBPF::receive(Iface& iface, const SocketInfo& socket_info) {
     // When using BPF, the read buffer must be allocated for the interface.
     // If it is not allocated, it is a programmatic error.
     if (iface.getReadBufferSize() == 0) {
-        isc_throw(SocketConfigError, "socket read buffer not allocated"
+        isc_throw(SocketConfigError, "socket read buffer empty"
                   " for the interface: " << iface.getName());
     }
 

diff --git a/src/lib/dhcp/pkt_filter_bpf.h b/src/lib/dhcp/pkt_filter_bpf.h
index bc9d80719ccf24ab844b82271a98c45822c05530..140ca0116d56c1d622ffc533fcd8161e0762cd80 100644 (file)
--- a/src/lib/dhcp/pkt_filter_bpf.h
+++ b/src/lib/dhcp/pkt_filter_bpf.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -109,7 +109,7 @@ public:
     /// @param socket_info structure holding socket information
     ///
     /// @return Received packet
-    virtual Pkt4Ptr receive(const Iface& iface, const SocketInfo& socket_info);
+    virtual Pkt4Ptr receive(Iface& iface, const SocketInfo& socket_info);
 
     /// @brief Send packet over specified socket.
     ///

diff --git a/src/lib/dhcp/pkt_filter_inet.cc b/src/lib/dhcp/pkt_filter_inet.cc
index 391aa13777ece5d4dc29fa184dcf058ae01e0419..468d0335eac268019b20a2d92e30e811271a34e4 100644 (file)
--- a/src/lib/dhcp/pkt_filter_inet.cc
+++ b/src/lib/dhcp/pkt_filter_inet.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -100,7 +100,7 @@ PktFilterInet::openSocket(Iface& iface,
 }
 
 Pkt4Ptr
-PktFilterInet::receive(const Iface& iface, const SocketInfo& socket_info) {
+PktFilterInet::receive(Iface& iface, const SocketInfo& socket_info) {
     struct sockaddr_in from_addr;
     uint8_t buf[IfaceMgr::RCVBUFSIZE];
 

diff --git a/src/lib/dhcp/pkt_filter_inet.h b/src/lib/dhcp/pkt_filter_inet.h
index 0c3bff433561276539d6e1fb58dfa60e3cf7e3e9..00ff070cfe4c108493ab0669454e425139f9f737 100644 (file)
--- a/src/lib/dhcp/pkt_filter_inet.h
+++ b/src/lib/dhcp/pkt_filter_inet.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -71,7 +71,7 @@ public:
     /// of the packet.
     /// @throw An execption thrown by the isc::dhcp::Pkt4 object if DHCPv4
     /// message parsing fails.
-    virtual Pkt4Ptr receive(const Iface& iface, const SocketInfo& socket_info);
+    virtual Pkt4Ptr receive(Iface& iface, const SocketInfo& socket_info);
 
     /// @brief Send packet over specified socket.
     ///

diff --git a/src/lib/dhcp/pkt_filter_lpf.cc b/src/lib/dhcp/pkt_filter_lpf.cc
index 595cd39ac290248349a4459a13029b4f21a4d54b..2959125e3461d2889a0ddd99eddcea57a4700858 100644 (file)
--- a/src/lib/dhcp/pkt_filter_lpf.cc
+++ b/src/lib/dhcp/pkt_filter_lpf.cc
@@ -201,7 +201,7 @@ PktFilterLPF::openSocket(Iface& iface,
 }
 
 Pkt4Ptr
-PktFilterLPF::receive(const Iface& iface, const SocketInfo& socket_info) {
+PktFilterLPF::receive(Iface& iface, const SocketInfo& socket_info) {
     uint8_t raw_buf[IfaceMgr::RCVBUFSIZE];
     // First let's get some data from the fallback socket. The data will be
     // discarded but we don't want the socket buffer to bloat. We get the

diff --git a/src/lib/dhcp/pkt_filter_lpf.h b/src/lib/dhcp/pkt_filter_lpf.h
index 304cbbd6dead1a15c42e7e9790c271b74701baad..ee3bb24619b4c2deccc76a6f0cdca74da229366f 100644 (file)
--- a/src/lib/dhcp/pkt_filter_lpf.h
+++ b/src/lib/dhcp/pkt_filter_lpf.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -62,7 +62,7 @@ public:
     ///
     /// @throw isc::NotImplemented always
     /// @return Received packet
-    virtual Pkt4Ptr receive(const Iface& iface, const SocketInfo& socket_info);
+    virtual Pkt4Ptr receive(Iface& iface, const SocketInfo& socket_info);
 
     /// @brief Send packet over specified socket.
     ///

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 2aecd276f57f1cb4103cab9db82bf1a90b1daca6..12f5a91f052151a178863d6a9ba6d610962ddbdf 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -151,8 +151,7 @@ public:
     }
 
     /// Does nothing
-    virtual Pkt4Ptr receive(const Iface&,
-                            const SocketInfo&) {
+    virtual Pkt4Ptr receive(Iface&, const SocketInfo&) {
         return (Pkt4Ptr());
     }
 

diff --git a/src/lib/dhcp/tests/pkt_filter_test_stub.cc b/src/lib/dhcp/tests/pkt_filter_test_stub.cc
index 0a38e173005ea072ebd35533ed83edc61e75992f..c3f2f84ac7abaecd192499ded65c303f0a6953f3 100644 (file)
--- a/src/lib/dhcp/tests/pkt_filter_test_stub.cc
+++ b/src/lib/dhcp/tests/pkt_filter_test_stub.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -35,7 +35,7 @@ PktFilterTestStub::openSocket(Iface&,
 }
 
 Pkt4Ptr
-PktFilterTestStub::receive(const Iface&, const SocketInfo&) {
+PktFilterTestStub::receive(Iface&, const SocketInfo&) {
     return Pkt4Ptr();
 }
 

diff --git a/src/lib/dhcp/tests/pkt_filter_test_stub.h b/src/lib/dhcp/tests/pkt_filter_test_stub.h
index f924ed5aa2f0f426d138274669cc2cf38b650788..6f8248f844ee2f5d8fb49e539e643f633f8148c0 100644 (file)
--- a/src/lib/dhcp/tests/pkt_filter_test_stub.h
+++ b/src/lib/dhcp/tests/pkt_filter_test_stub.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014, 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -77,7 +77,7 @@ public:
     /// @note All parameters are ignored.
     ///
     /// @return always a NULL object.
-    virtual Pkt4Ptr receive(const Iface& iface, const SocketInfo& sock_info);
+    virtual Pkt4Ptr receive(Iface& iface, const SocketInfo& sock_info);
 
     /// @brief Simulates sending a DHCPv4 message.
     ///

diff --git a/src/lib/dhcp/tests/pkt_filter_test_utils.cc b/src/lib/dhcp/tests/pkt_filter_test_utils.cc
index 82d574ca4efc3b910acff8fc9569ae5e300094ad..526eb03a79c3dc2756c6081aeb4b1ae3aab09af8 100644 (file)
--- a/src/lib/dhcp/tests/pkt_filter_test_utils.cc
+++ b/src/lib/dhcp/tests/pkt_filter_test_utils.cc
@@ -180,7 +180,7 @@ PktFilterStub::openSocket(Iface&,
 }
 
 Pkt4Ptr
-PktFilterStub::receive(const Iface&, const SocketInfo&) {
+PktFilterStub::receive(Iface&, const SocketInfo&) {
     return Pkt4Ptr();
 }
 

diff --git a/src/lib/dhcp/tests/pkt_filter_test_utils.h b/src/lib/dhcp/tests/pkt_filter_test_utils.h
index 691d3a17635a898ccd6374f0265a2ac4e565ff7d..40a1e020e1d443bb09afd5d8f2a64f36b54aa6d6 100644 (file)
--- a/src/lib/dhcp/tests/pkt_filter_test_utils.h
+++ b/src/lib/dhcp/tests/pkt_filter_test_utils.h
@@ -144,7 +144,7 @@ public:
     /// @note All parameters are ignored.
     ///
     /// @return always a NULL object.
-    virtual Pkt4Ptr receive(const Iface& iface, const SocketInfo& sock_info);
+    virtual Pkt4Ptr receive(Iface& iface, const SocketInfo& sock_info);
 
     /// @brief Simulates sending a DHCPv4 message.
     ///