[libvirt] [PATCH] Fix libvirtd crash possibility

[Eric Blake]

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.

> 
> The problem is that the skipped numbers will get func filled with
> NULLs, but there is no check whether these are set before the daemon
> tries to run them. This patch very simply enhances one check and fixes
> that.
> 
> I haven't investigated into such a deepness to say if the possibility
> comes as well with hacked RPC call (calling event instead of
> function), but this patch gets rid of both these problems.

You may want to modify this paragraph before pushing, perhaps based on
my details above (and feel free to ask me on IRC if you need a better
understanding of the rules of what RPC numbers can and can't be backported).

> ---
>  src/rpc/virnetserverprogram.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
> index d13b621..bc85df0 100644
> --- a/src/rpc/virnetserverprogram.c
> +++ b/src/rpc/virnetserverprogram.c
> @@ -1,7 +1,7 @@
>  /*
>   * virnetserverprogram.c: generic network RPC server program
>   *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -379,7 +379,7 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog,
> 
>      dispatcher = virNetServerProgramGetProc(prog, msg->header.proc);
> 
> -    if (!dispatcher) {
> +    if (!dispatcher || !dispatcher->func) {
>          virReportError(VIR_ERR_RPC,
>                         _("unknown procedure: %d"),
>                         msg->header.proc);

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.

-- 
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/50ce6732/attachment-0001.sig>