[libvirt] Bug in RPC code causes failure to start LXC container using virDomainCreateXMLWithFiles

[Ben Gray]

Hi,

     Occasionally when trying to start LXC containers with fds I get the 
following error:

         virNetMessageDupFD:562 : Unable to duplicate FD -1: Bad file 
descriptor

     I tracked it down to the code that handles EAGAIN errors from 
recvfd.  In such cases the virNetMessageDecodeNumFDs function may be 
called multiple times from virNetServerClientDispatchRead and each time 
it overwrites the msg->fds array.  In the best case (when msg->donefds 
== 0) this results in a memory leak, in the worse case it will leak any 
fd's already in msg->fds and cause subsequent failures when dup is called.

     A very similar problem is mention here: 
https://www.redhat.com/archives/libvir-list/2012-December/msg01306.html


     Below is my patch to fix the issue.



--- a/src/rpc/virnetserverclient.c    2015-01-23 11:46:24.000000000 +0000
+++ b/src/rpc/virnetserverclient.c    2015-11-26 15:30:51.214462290 +0000
@@ -1107,36 +1107,40 @@

          /* 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 */
-        }
+        if (msg->header.type == VIR_NET_CALL_WITH_FDS) {
+            size_t i;

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