From 133c511b5275cfd0a024fe0c62467f494dc62e40 Mon Sep 17 00:00:00 2001 From: Ben Gray Date: Thu, 26 Nov 2015 16:10:40 +0000 Subject: [PATCH] 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 --- src/rpc/virnetserverclient.c | 52 +++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 25 deletions(-) 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; /* 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) { + 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; } - 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 */