[libvirt] [PATCH v2 4/4] libvirt-<module>: Check caller-provided buffers to be NULL with size > 0

[Daniel P. Berrangé]

On Thu, Nov 21, 2019 at 09:58:32AM +0100, Erik Skultety wrote:
> Pre-Glib era which used malloc allowed the size of the client-side
> buffers to be declared as 0, because malloc documents that it can either
> return 0 or a unique pointer on 0 size allocations.
> With glib this doesn't work anymore, because glib documents that for
> such allocation requests NULL is always returned which results in an
> error in our public API checks server-side.
> This patch complements the fix in the RPC layer by explicitly erroring
> out on the following combination of args used by our legacy APIs (their
> moder equivalents don't suffer from this):
> 
> function(caller-allocated-array, size, ...) {
>     if (!caller-allocated-array && size > 0)
>         return error;
> }
> 
> treating everything else as a valid input and potentially let that fail
> on the server-side rather than client-side.
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/internal.h                | 13 +++++++++++++
>  src/libvirt-domain-snapshot.c |  4 ++--
>  src/libvirt-domain.c          | 21 ++++++---------------
>  src/libvirt-host.c            |  2 +-
>  src/libvirt-interface.c       |  4 ++--
>  src/libvirt-network.c         |  4 ++--
>  src/libvirt-nodedev.c         |  4 ++--
>  src/libvirt-nwfilter.c        |  2 +-
>  src/libvirt-secret.c          |  2 +-
>  src/libvirt-storage.c         |  6 +++---
>  10 files changed, 33 insertions(+), 29 deletions(-)


> @@ -11136,10 +11133,7 @@ virDomainFSFreeze(virDomainPtr dom,
>  
>      virCheckDomainReturn(dom, -1);
>      virCheckReadOnlyGoto(dom->conn->flags, error);
> -    if (nmountpoints)
> -        virCheckNonNullArgGoto(mountpoints, error);
> -    else
> -        virCheckNullArgGoto(mountpoints, error);

Interesting, so this actually returned an error if you did *not*
pass NULL when nmountpoints == 0, so this would be broken if
the caller's malloc() returned non-NULL region for size==0

> +    virCheckNonNullArrayArgGoto(mountpoints, nmountpoints, error);
>  

Reviewed-by: Daniel P. Berrangé <berrange at redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|