[libvirt] [PATCH] rpc: Fix crash on error paths of message dispatching

[Guido Günther]

Hi,
On Mon, Jan 28, 2013 at 07:35:38PM +0100, Peter Krempa wrote:
> When reading and dispatching of a message failed the message was freed
> but wasn't removed from the message queue.
> 
> After that when the connection was about to be closed the pointer for
> the message was still present in the queue and it was passed to
> virNetMessageFree which tried to call the callback function from an
> uninitialized pointer.

Debian stable is shipping 0.8.2. I checked and it seems this version
isn't affected siince we properly remove the message from the queue
before looking at it in daemon/libvirtd.c. I'd be great if somebody
could double check though!
Cheers,
 -- Guido

> 
> This patch removes the message from the queue before it's freed.
> 
> * rpc/virnetserverclient.c: virNetServerClientDispatchRead:
>     - avoid use after free of RPC messages
> ---
>  src/rpc/virnetserverclient.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index af0560e..446e1e9 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -987,6 +987,7 @@ readmore:
> 
>          /* Decode the header so we can use it for routing decisions */
>          if (virNetMessageDecodeHeader(msg) < 0) {
> +            virNetMessageQueueServe(&client->rx);
>              virNetMessageFree(msg);
>              client->wantClose = true;
>              return;
> @@ -996,6 +997,7 @@ readmore:
>           * file descriptors */
>          if (msg->header.type == VIR_NET_CALL_WITH_FDS &&
>              virNetMessageDecodeNumFDs(msg) < 0) {
> +            virNetMessageQueueServe(&client->rx);
>              virNetMessageFree(msg);
>              client->wantClose = true;
>              return; /* Error */
> @@ -1005,6 +1007,7 @@ readmore:
>          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;
> -- 
> 1.8.1.1
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>