Fix socket cmsg buffer usage

(cherry picked from commit d79be7dd5e10fae87cca200b7950f8c9c3b52096)
(cherry picked from commit da63e956124eb1e56b7bd302f5769cf61e4f8dba)

diff --git a/CHANGES b/CHANGES
index 2b02ea2e47f22dead1270a7354edb6922fa4050f..7713b7af834eead0430fce44005a931b3659f6cb 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+4981.  [bug]           Fix race in cmsg buffer usage in socket code.
+                       [GL #180]
+
 4980.  [bug]           Named-checkconf failed to detect bad in-view targets.
                        [GL #288]
 

diff --git a/lib/isc/unix/socket.c b/lib/isc/unix/socket.c
index 2a2a5b08efb54be972dc97c1d52183cb1bcd154d..1375b6534163e74d56142285c9d8096073ef3572 100644 (file)
--- a/lib/isc/unix/socket.c
+++ b/lib/isc/unix/socket.c
@@ -374,9 +374,7 @@ struct isc__socket {
        unsigned char           overflow; /* used for MSG_TRUNC fake */
 #endif
 
-       char                    *recvcmsgbuf;
        ISC_SOCKADDR_LEN_T      recvcmsgbuflen;
-       char                    *sendcmsgbuf;
        ISC_SOCKADDR_LEN_T      sendcmsgbuflen;
 
        void                    *fdwatcharg;
@@ -485,9 +483,9 @@ static void internal_send(isc_task_t *, isc_event_t *);
 static void internal_fdwatch_write(isc_task_t *, isc_event_t *);
 static void internal_fdwatch_read(isc_task_t *, isc_event_t *);
 static void process_cmsg(isc__socket_t *, struct msghdr *, isc_socketevent_t *);
-static void build_msghdr_send(isc__socket_t *, isc_socketevent_t *,
+static void build_msghdr_send(isc__socket_t *, char *, isc_socketevent_t *,
                              struct msghdr *, struct iovec *, size_t *);
-static void build_msghdr_recv(isc__socket_t *, isc_socketevent_t *,
+static void build_msghdr_recv(isc__socket_t *, char *, isc_socketevent_t *,
                              struct msghdr *, struct iovec *, size_t *);
 #ifdef USE_WATCHER_THREAD
 static isc_boolean_t process_ctlfd(isc__socketmgr_t *manager);
@@ -1441,7 +1439,7 @@ process_cmsg(isc__socket_t *sock, struct msghdr *msg, isc_socketevent_t *dev) {
  * this transaction can send.
  */
 static void
-build_msghdr_send(isc__socket_t *sock, isc_socketevent_t *dev,
+build_msghdr_send(isc__socket_t *sock, char* cmsgbuf, isc_socketevent_t *dev,
                  struct msghdr *msg, struct iovec *iov, size_t *write_countp)
 {
        unsigned int iovcount;
@@ -1454,8 +1452,9 @@ build_msghdr_send(isc__socket_t *sock, isc_socketevent_t *dev,
 #endif
 
        memset(msg, 0, sizeof(*msg));
+
        if (sock->sendcmsgbuflen != 0U) {
-               memset(sock->sendcmsgbuf, 0, sock->sendcmsgbuflen);
+               memset(cmsgbuf, 0, sock->sendcmsgbuflen);
        }
 
        if (!sock->connected) {
@@ -1534,11 +1533,11 @@ build_msghdr_send(isc__socket_t *sock, isc_socketevent_t *dev,
                           "sendto pktinfo data, ifindex %u",
                           dev->pktinfo.ipi6_ifindex);
 
+               msg->msg_control = (void *)cmsgbuf;
                msg->msg_controllen = cmsg_space(sizeof(struct in6_pktinfo));
                INSIST(msg->msg_controllen <= sock->sendcmsgbuflen);
-               msg->msg_control = (void *)sock->sendcmsgbuf;
 
-               cmsgp = (struct cmsghdr *)sock->sendcmsgbuf;
+               cmsgp = (struct cmsghdr *)cmsgbuf;
                cmsgp->cmsg_level = IPPROTO_IPV6;
                cmsgp->cmsg_type = IPV6_PKTINFO;
                cmsgp->cmsg_len = cmsg_len(sizeof(struct in6_pktinfo));
@@ -1553,7 +1552,7 @@ build_msghdr_send(isc__socket_t *sock, isc_socketevent_t *dev,
        {
                int use_min_mtu = 1;    /* -1, 0, 1 */
 
-               cmsgp = (struct cmsghdr *)(sock->sendcmsgbuf +
+               cmsgp = (struct cmsghdr *)(cmsgbuf +
                                           msg->msg_controllen);
                msg->msg_controllen += cmsg_space(sizeof(use_min_mtu));
                INSIST(msg->msg_controllen <= sock->sendcmsgbuflen);
@@ -1582,9 +1581,9 @@ build_msghdr_send(isc__socket_t *sock, isc_socketevent_t *dev,
 
 #ifdef IP_TOS
                if (sock->pf == AF_INET && sock->pktdscp) {
-                       cmsgp = (struct cmsghdr *)(sock->sendcmsgbuf +
+                       cmsgp = (struct cmsghdr *)(cmsgbuf +
                                                   msg->msg_controllen);
-                       msg->msg_control = (void *)sock->sendcmsgbuf;
+                       msg->msg_control = (void *)cmsgbuf;
                        msg->msg_controllen += cmsg_space(sizeof(dscp));
                        INSIST(msg->msg_controllen <= sock->sendcmsgbuflen);
 
@@ -1613,9 +1612,9 @@ build_msghdr_send(isc__socket_t *sock, isc_socketevent_t *dev,
 #endif
 #if defined(IPPROTO_IPV6) && defined(IPV6_TCLASS)
                if (sock->pf == AF_INET6 && sock->pktdscp) {
-                       cmsgp = (struct cmsghdr *)(sock->sendcmsgbuf +
+                       cmsgp = (struct cmsghdr *)(cmsgbuf +
                                                   msg->msg_controllen);
-                       msg->msg_control = (void *)sock->sendcmsgbuf;
+                       msg->msg_control = (void *)cmsgbuf;
                        msg->msg_controllen += cmsg_space(sizeof(dscp));
                        INSIST(msg->msg_controllen <= sock->sendcmsgbuflen);
 
@@ -1641,6 +1640,12 @@ build_msghdr_send(isc__socket_t *sock, isc_socketevent_t *dev,
                                sock->dscp = dscp;
                }
 #endif
+               if (msg->msg_controllen != 0 &&
+                   msg->msg_controllen < sock->sendcmsgbuflen)
+               {
+                       memset(cmsgbuf + msg->msg_controllen, 0,
+                              sock->sendcmsgbuflen - msg->msg_controllen);
+               }
        }
 #endif
 #endif /* USE_CMSG */
@@ -1666,7 +1671,7 @@ build_msghdr_send(isc__socket_t *sock, isc_socketevent_t *dev,
  * this transaction can receive.
  */
 static void
-build_msghdr_recv(isc__socket_t *sock, isc_socketevent_t *dev,
+build_msghdr_recv(isc__socket_t *sock, char *cmsgbuf, isc_socketevent_t *dev,
                  struct msghdr *msg, struct iovec *iov, size_t *read_countp)
 {
        unsigned int iovcount;
@@ -1764,7 +1769,7 @@ build_msghdr_recv(isc__socket_t *sock, isc_socketevent_t *dev,
 
 #ifdef ISC_NET_BSD44MSGHDR
 #if defined(USE_CMSG)
-       msg->msg_control = sock->recvcmsgbuf;
+       msg->msg_control = cmsgbuf;
        msg->msg_controllen = sock->recvcmsgbuflen;
 #else
        msg->msg_control = NULL;
@@ -1868,8 +1873,9 @@ doio_recv(isc__socket_t *sock, isc_socketevent_t *dev) {
        isc_buffer_t *buffer;
        int recv_errno;
        char strbuf[ISC_STRERRORSIZE];
+       char cmsgbuf[sock->recvcmsgbuflen];
 
-       build_msghdr_recv(sock, dev, &msghdr, iov, &read_count);
+       build_msghdr_recv(sock, cmsgbuf, dev, &msghdr, iov, &read_count);
 
 #if defined(ISC_SOCKET_DEBUG)
        dump_msg(&msghdr);
@@ -2063,8 +2069,9 @@ doio_send(isc__socket_t *sock, isc_socketevent_t *dev) {
        int attempts = 0;
        int send_errno;
        char strbuf[ISC_STRERRORSIZE];
+       char cmsgbuf[sock->sendcmsgbuflen];
 
-       build_msghdr_send(sock, dev, &msghdr, iov, &write_count);
+       build_msghdr_send(sock, cmsgbuf, dev, &msghdr, iov, &write_count);
 
  resend:
        if (sock->type == isc_sockettype_udp &&
@@ -2303,11 +2310,8 @@ allocate_socket(isc__socketmgr_t *manager, isc_sockettype_t type,
 
        ISC_LINK_INIT(sock, link);
 
-       sock->recvcmsgbuf = NULL;
-       sock->sendcmsgbuf = NULL;
-
        /*
-        * Set up cmsg buffers.
+        * Set up cmsg buffer lengths.
         */
        cmsgbuflen = 0;
 #if defined(USE_CMSG) && defined(ISC_PLATFORM_HAVEIN6PKTINFO)
@@ -2320,13 +2324,6 @@ allocate_socket(isc__socketmgr_t *manager, isc_sockettype_t type,
        cmsgbuflen += cmsg_space(sizeof(int));
 #endif
        sock->recvcmsgbuflen = cmsgbuflen;
-       if (sock->recvcmsgbuflen != 0U) {
-               sock->recvcmsgbuf = isc_mem_get(manager->mctx, cmsgbuflen);
-               if (sock->recvcmsgbuf == NULL) {
-                       result = ISC_R_NOMEMORY;
-                       goto error;
-               }
-       }
 
        cmsgbuflen = 0;
 #if defined(USE_CMSG) && defined(ISC_PLATFORM_HAVEIN6PKTINFO)
@@ -2343,13 +2340,6 @@ allocate_socket(isc__socketmgr_t *manager, isc_sockettype_t type,
        cmsgbuflen += cmsg_space(sizeof(int));
 #endif
        sock->sendcmsgbuflen = cmsgbuflen;
-       if (sock->sendcmsgbuflen != 0U) {
-               sock->sendcmsgbuf = isc_mem_get(manager->mctx, cmsgbuflen);
-               if (sock->sendcmsgbuf == NULL) {
-                       result = ISC_R_NOMEMORY;
-                       goto error;
-               }
-       }
 
        memset(sock->name, 0, sizeof(sock->name));
        sock->tag = NULL;
@@ -2397,12 +2387,6 @@ allocate_socket(isc__socketmgr_t *manager, isc_sockettype_t type,
        return (ISC_R_SUCCESS);
 
  error:
-       if (sock->recvcmsgbuf != NULL)
-               isc_mem_put(manager->mctx, sock->recvcmsgbuf,
-                           sock->recvcmsgbuflen);
-       if (sock->sendcmsgbuf != NULL)
-               isc_mem_put(manager->mctx, sock->sendcmsgbuf,
-                           sock->sendcmsgbuflen);
        isc_mem_put(manager->mctx, sock, sizeof(*sock));
 
        return (result);
@@ -2430,13 +2414,6 @@ free_socket(isc__socket_t **socketp) {
        INSIST(ISC_LIST_EMPTY(sock->accept_list));
        INSIST(!ISC_LINK_LINKED(sock, link));
 
-       if (sock->recvcmsgbuf != NULL)
-               isc_mem_put(sock->manager->mctx, sock->recvcmsgbuf,
-                           sock->recvcmsgbuflen);
-       if (sock->sendcmsgbuf != NULL)
-               isc_mem_put(sock->manager->mctx, sock->sendcmsgbuf,
-                           sock->sendcmsgbuflen);
-
        sock->common.magic = 0;
        sock->common.impmagic = 0;