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

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.

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();
     }
 

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;
 

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

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.

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());
     }
 

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.
     ///

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];
 

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.
     ///

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

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.
     ///

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());
     }
 

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();
 }
 

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.
     ///

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();
 }
 

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.
     ///