After libvirt switched to GLib, we also started to use glib allocation
primitives as of commit e85e34f3. Unlike malloc which is ambiguous with
regards to size == 0 (which in our case returned a unique pointer safe
to be passed to free), g_malloc0 strictly returns NULL on size == 0.

This change broke our legacy APIs which rely on the caller to
pre-allocate the target buffer to hold the results and pass the buffer
size as an argument. Since it makes very little sense to call an API
with nowhere to store the results, fix this by returning 0 directly in
such case in the RPC dispatch code - there are modern API equivalents
allocating the target buffer automatically anyway.

https://bugzilla.redhat.com/show_bug.cgi?id=1772842

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 src/rpc/gendispatch.pl | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 8656c8f205..524d31f741 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1073,6 +1073,14 @@ elsif ($mode eq "server") {
             print "        goto cleanup;\n";
             print "    }\n";
             print "\n";
+
+
+            print "    if (args->$single_ret_list_max_var == 0) {\n";
+            print "        ret->$single_ret_list_name.${single_ret_list_name}_len = 0;\n";
+            print "        rv = 0;\n";
+            print "        goto cleanup;\n";
+            print "    }\n";
+            print "\n";
         }
 
         print join("\n", @getters_list);
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Nov 21, 2019 at 09:58:30AM +0100, Erik Skultety wrote:
> After libvirt switched to GLib, we also started to use glib allocation
> primitives as of commit e85e34f3. Unlike malloc which is ambiguous with
> regards to size == 0 (which in our case returned a unique pointer safe
> to be passed to free), g_malloc0 strictly returns NULL on size == 0.
> 
> This change broke our legacy APIs which rely on the caller to
> pre-allocate the target buffer to hold the results and pass the buffer
> size as an argument. Since it makes very little sense to call an API
> with nowhere to store the results, fix this by returning 0 directly in
> such case in the RPC dispatch code - there are modern API equivalents
> allocating the target buffer automatically anyway.

As mentioned before, I don't think we should be short-circuiting the
driver APIs like this. It is still valid to call the APIs with zero
length list such that we get access control checks validated.

> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1772842
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/rpc/gendispatch.pl | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
> index 8656c8f205..524d31f741 100755
> --- a/src/rpc/gendispatch.pl
> +++ b/src/rpc/gendispatch.pl
> @@ -1073,6 +1073,14 @@ elsif ($mode eq "server") {
>              print "        goto cleanup;\n";
>              print "    }\n";
>              print "\n";
> +
> +
> +            print "    if (args->$single_ret_list_max_var == 0) {\n";
> +            print "        ret->$single_ret_list_name.${single_ret_list_name}_len = 0;\n";
> +            print "        rv = 0;\n";
> +            print "        goto cleanup;\n";
> +            print "    }\n";
> +            print "\n";
>          }
>  
>          print join("\n", @getters_list);
> -- 
> 2.23.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list