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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to