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

[Guido Günther]

Hi Eric,
On Tue, Jan 29, 2013 at 02:21:30PM -0700, Eric Blake wrote:
> On 01/29/2013 01:22 PM, Guido Günther wrote:
> > 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!
> 
> 0.8.2 predates the RPC rewrite, and I concur with your assessment that
> back then, the code was _always_ clearing the queue:
> 
> v0.8.2:daemon/libvirtd.c:qemudDispatchClientRead():
> 
>         /* Grab the completed message */
>         struct qemud_client_message *msg =
> qemudClientMessageQueueServe(&client->rx);
>         struct qemud_client_filter *filter;
> 
>         /* Decode the header so we can use it for routing decisions */
>         if (remoteDecodeClientMessageHeader(msg) < 0) {
>             VIR_FREE(msg);
>             qemudDispatchClientFailure(client);
>         }
> 
> However, it does look like there might be a missing 'return' statement
> after that error is reported, especially given that the next error
> reporting a few lines later does an early return.

Thanks for double checking. It indeed looks like there's a return
missing (cc:'ing the Debian bugreport to make this information permanent
there too).
Cheers,
 -- Guido

> But the best way to determine if this version is actually vulnerable to
> the CVE would be trying the exploit, and seeing if libvirtd survives
> with proper error logging about an invalid client request; Peter may
> have more details on how best to attempt that (although it may be better
> to discuss those details off-list, even if the CVE is already public, so
> that others are less likely to maliciously use the exploit).


> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>