[libvirt] [PATCH] Fix libvirtd crash possibility

[Eric Blake]

On 09/12/2012 04:07 PM, Eric Blake wrote:
> On 09/12/2012 03:54 PM, Martin Kletzander wrote:
>> When generating RPC protocol messages, it's strictly needed to have
>> continuousline of numbers or RPC messages. However in case anyone
>> tries backporting some functionality and will skip a number, there is
>> a possibility to make the daemon segfault with newer virsh (version of
>> the library, rpc call, etc.) even unintentionally.
> 
> Indeed, upstream is contiguous, but downstream, we _can't_ backport new
> calls (because that would break .so versioning), but _must_ backport new
> events using the same RPC number as upstream (since events don't break
> .so), so downstream can definitely get into a state of gaps in the
> numbering.  Although upstream will never have the gap, I think a patch
> along these lines is appropriate for upstream, rather than forcing each
> downstream to deal with the gap they create by selective backports.

Correcting myself, there are gaps even in upstream - we MUST assume
malicious clients, that try to invoke an event, and therefore this can
crash upstream libvirtd without any backporting involved if it is not
patched.

Looking at the generated daemon/remote_dispatch.h, we can see things like:

{ /* Async event DomainEventBalloonChange => 276 */
   NULL,
   0,
   (xdrproc_t)xdr_void,
   0,
   (xdrproc_t)xdr_void,
   true,
   0
},
{ /* Method DomainGetHostname => 277 */
   remoteDispatchDomainGetHostnameHelper,
   sizeof(remote_domain_get_hostname_args),
   (xdrproc_t)xdr_remote_domain_get_hostname_args,
   sizeof(remote_domain_get_hostname_ret),
   (xdrproc_t)xdr_remote_domain_get_hostname_ret,
   true,
   0
},

where a malicious client trying to call RPC 276 will trigger the failure
if we don't patch things.

> 
> Not quite right.  I think the correct fix is to instead make
> virNetServerProgramGetProc() return NULL instead of an uninitialized
> entry, if that entry has a NULL func pointer.

I still think my suggestion is correct, though - fix it at the source,
not at each caller.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120912/8bdc2d51/attachment-0001.sig>