From: Ben Gray <ben.r.gray@gmail.com>
Date: Thu, 26 Nov 2015 16:10:40 +0000 (+0000)
Subject: rpc: Don't rewrite msg->fds on every read dispatch
X-Git-Tag: v1.3.1-rc2~24
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=133c511b5275cfd0a024fe0c62467f494dc62e40;p=thirdparty%2Flibvirt.git

rpc: Don't rewrite msg->fds on every read dispatch

When we are receiving data in smaller chunks it might happen that
virNetServerClientDispatchRead() will be called multiple times.  And as
that happens, if it is a message that also transfer headers, we decode
the number of them every single time and, unfortunately, also allocate
the memory for them.  That causes a leak, in the best scenario.

Best viewed with '-w'.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 0e3a71f9b2..64dab46314 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1107,36 +1107,38 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client)
 
         /* Now figure out if we need to read more data to get some
          * file descriptors */
-        if (msg->header.type == VIR_NET_CALL_WITH_FDS &&
-            virNetMessageDecodeNumFDs(msg) < 0) {
-            virNetMessageQueueServe(&client->rx);
-            virNetMessageFree(msg);
-            client->wantClose = true;
-            return; /* Error */
-        }
-
-        /* Try getting the file descriptors (may fail if blocking) */
-        for (i = msg->donefds; i < msg->nfds; i++) {
-            int rv;
-            if ((rv = virNetSocketRecvFD(client->sock, &(msg->fds[i]))) < 0) {
+        if (msg->header.type == VIR_NET_CALL_WITH_FDS) {
+            if (msg->nfds == 0 &&
+                virNetMessageDecodeNumFDs(msg) < 0) {
                 virNetMessageQueueServe(&client->rx);
                 virNetMessageFree(msg);
                 client->wantClose = true;
-                return;
+                return; /* Error */
             }
-            if (rv == 0) /* Blocking */
-                break;
-            msg->donefds++;
-        }
 
-        /* Need to poll() until FDs arrive */
-        if (msg->donefds < msg->nfds) {
-            /* Because DecodeHeader/NumFDs reset bufferOffset, we
-             * put it back to what it was, so everything works
-             * again next time we run this method
-             */
-            client->rx->bufferOffset = client->rx->bufferLength;
-            return;
+            /* Try getting the file descriptors (may fail if blocking) */
+            for (i = msg->donefds; i < msg->nfds; i++) {
+                int rv;
+                if ((rv = virNetSocketRecvFD(client->sock, &(msg->fds[i]))) < 0) {
+                    virNetMessageQueueServe(&client->rx);
+                    virNetMessageFree(msg);
+                    client->wantClose = true;
+                    return;
+                }
+                if (rv == 0) /* Blocking */
+                    break;
+                msg->donefds++;
+            }
+
+            /* Need to poll() until FDs arrive */
+            if (msg->donefds < msg->nfds) {
+                /* Because DecodeHeader/NumFDs reset bufferOffset, we
+                 * put it back to what it was, so everything works
+                 * again next time we run this method
+                 */
+                client->rx->bufferOffset = client->rx->bufferLength;
+                return;
+            }
         }
 
         /* Definitely finished reading, so remove from queue */