libvirt-<module>: Check caller-provided buffers to be NULL with size > 0

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.

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

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/internal.h b/src/internal.h
index 0ff9f496ace62e436bf2392488e0bfa5a30b967d..bcc5a1c1579738abb57e78ca3b7cf3fbef9b3684 100644 (file)
--- a/src/internal.h
+++ b/src/internal.h
@@ -429,6 +429,19 @@
         } \
     } while (0)
 
+/* This check is intended to be used with legacy APIs only which expect the
+ * caller to pre-allocate the target buffer.
+ * We want to allow callers pass NULL arrays if the size is declared as 0 and
+ * still succeed in calling the API.
+ */
+#define virCheckNonNullArrayArgGoto(argname, argsize, label) \
+    do { \
+        if (!argname && argsize > 0) { \
+            virReportInvalidNonNullArg(argname); \
+            goto label; \
+        } \
+    } while (0)
+
 
 /* Count leading zeros in an unsigned int.
  *

diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 20a3bc5545b2e16863056015c1cc3c909c66d4dd..33593e11e90816d5fe0ea28658544e49c14157f5 100644 (file)
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -398,7 +398,7 @@ virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen,
     virCheckDomainReturn(domain, -1);
     conn = domain->conn;
 
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, nameslen, error);
     virCheckNonNegativeArgGoto(nameslen, error);
 
     if (conn->driver->domainSnapshotListNames) {
@@ -600,7 +600,7 @@ virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
     virCheckDomainSnapshotReturn(snapshot, -1);
     conn = snapshot->domain->conn;
 
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, nameslen, error);
     virCheckNonNegativeArgGoto(nameslen, error);
 
     if (conn->driver->domainSnapshotListChildrenNames) {

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2d9c4061e2b702180f2bdfd0579255f7f9d50c50..51fb79cdddab7e254ae23845607f65178ebd5d63 100644 (file)
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -59,7 +59,7 @@ virConnectListDomains(virConnectPtr conn, int *ids, int maxids)
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(ids, error);
+    virCheckNonNullArrayArgGoto(ids, maxids, error);
     virCheckNonNegativeArgGoto(maxids, error);
 
     if (conn->driver->connectListDomains) {
@@ -6386,7 +6386,7 @@ virConnectListDefinedDomains(virConnectPtr conn, char **const names,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->driver->connectListDefinedDomains) {
@@ -7298,7 +7298,7 @@ virDomainGetVcpuPinInfo(virDomainPtr domain, int ncpumaps,
     virCheckDomainReturn(domain, -1);
     conn = domain->conn;
 
-    virCheckNonNullArgGoto(cpumaps, error);
+    virCheckNonNullArrayArgGoto(cpumaps, ncpumaps, error);
     virCheckPositiveArgGoto(ncpumaps, error);
     virCheckPositiveArgGoto(maplen, error);
 
@@ -10996,10 +10996,7 @@ virDomainGetDiskErrors(virDomainPtr dom,
 
     virCheckDomainReturn(dom, -1);
 
-    if (maxerrors)
-        virCheckNonNullArgGoto(errors, error);
-    else
-        virCheckNullArgGoto(errors, error);
+    virCheckNonNullArrayArgGoto(errors, maxerrors, error);
 
     if (dom->conn->driver->domainGetDiskErrors) {
         int ret = dom->conn->driver->domainGetDiskErrors(dom, errors,
@@ -11136,10 +11133,7 @@ virDomainFSFreeze(virDomainPtr dom,
 
     virCheckDomainReturn(dom, -1);
     virCheckReadOnlyGoto(dom->conn->flags, error);
-    if (nmountpoints)
-        virCheckNonNullArgGoto(mountpoints, error);
-    else
-        virCheckNullArgGoto(mountpoints, error);
+    virCheckNonNullArrayArgGoto(mountpoints, nmountpoints, error);
 
     if (dom->conn->driver->domainFSFreeze) {
         int ret = dom->conn->driver->domainFSFreeze(
@@ -11181,10 +11175,7 @@ virDomainFSThaw(virDomainPtr dom,
 
     virCheckDomainReturn(dom, -1);
     virCheckReadOnlyGoto(dom->conn->flags, error);
-    if (nmountpoints)
-        virCheckNonNullArgGoto(mountpoints, error);
-    else
-        virCheckNullArgGoto(mountpoints, error);
+    virCheckNonNullArrayArgGoto(mountpoints, nmountpoints, error);
 
     if (dom->conn->driver->domainFSThaw) {
         int ret = dom->conn->driver->domainFSThaw(

diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index 221a1b7a4353a80e7b0c0cf99003f863f81cb05a..94ba5a8e80f54a773c750bc6055bc9e68a1c2aae 100644 (file)
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -910,7 +910,7 @@ virNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freeMems,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(freeMems, error);
+    virCheckNonNullArrayArgGoto(freeMems, maxCells, error);
     virCheckPositiveArgGoto(maxCells, error);
     virCheckNonNegativeArgGoto(startCell, error);
 

diff --git a/src/libvirt-interface.c b/src/libvirt-interface.c
index 7228ddca574769e8fee0f442112be2b603154dd2..2d2df68131a505497b6c19c19d460fb4dce66404 100644 (file)
--- a/src/libvirt-interface.c
+++ b/src/libvirt-interface.c
@@ -166,7 +166,7 @@ virConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames)
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->interfaceDriver && conn->interfaceDriver->connectListInterfaces) {
@@ -245,7 +245,7 @@ virConnectListDefinedInterfaces(virConnectPtr conn,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->interfaceDriver && conn->interfaceDriver->connectListDefinedInterfaces) {

diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index 146ccc5e4a3c7eb24a8d66147e9c286d6b55111e..09e24fb0a874bc53074e84718b488524e7f347ab 100644 (file)
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -175,7 +175,7 @@ virConnectListNetworks(virConnectPtr conn, char **const names, int maxnames)
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->networkDriver && conn->networkDriver->connectListNetworks) {
@@ -252,7 +252,7 @@ virConnectListDefinedNetworks(virConnectPtr conn, char **const names,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->networkDriver && conn->networkDriver->connectListDefinedNetworks) {

diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index 10050b193b38851926c695936fe2a0e8ac71ea78..dce46b71817545e1de82ce5df9647482cb0f1456 100644 (file)
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -169,7 +169,7 @@ virNodeListDevices(virConnectPtr conn,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->nodeDeviceDriver && conn->nodeDeviceDriver->nodeListDevices) {
@@ -415,7 +415,7 @@ virNodeDeviceListCaps(virNodeDevicePtr dev,
     virResetLastError();
 
     virCheckNodeDeviceReturn(dev, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (dev->conn->nodeDeviceDriver && dev->conn->nodeDeviceDriver->nodeDeviceListCaps) {

diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c
index 16eceb6525b717f0db140003bd9182c90da6c7cd..d28220db8a79969fbff1bbb1efe6537c6b8bfd9b 100644 (file)
--- a/src/libvirt-nwfilter.c
+++ b/src/libvirt-nwfilter.c
@@ -127,7 +127,7 @@ virConnectListNWFilters(virConnectPtr conn, char **const names, int maxnames)
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->nwfilterDriver && conn->nwfilterDriver->connectListNWFilters) {

diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c
index 711c4fc580377ced054a933bec78757f24927331..33cbdd7b0bb2c95a52c41d900c2d2678b314383b 100644 (file)
--- a/src/libvirt-secret.c
+++ b/src/libvirt-secret.c
@@ -166,7 +166,7 @@ virConnectListSecrets(virConnectPtr conn, char **uuids, int maxuuids)
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(uuids, error);
+    virCheckNonNullArrayArgGoto(uuids, maxuuids, error);
     virCheckNonNegativeArgGoto(maxuuids, error);
 
     if (conn->secretDriver != NULL && conn->secretDriver->connectListSecrets != NULL) {

diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 05b23656925bbaf14656ee34823a68c81a4d6168..0406fe84d3c212b2a032135638d1b23a57237076 100644 (file)
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -197,7 +197,7 @@ virConnectListStoragePools(virConnectPtr conn,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->storageDriver && conn->storageDriver->connectListStoragePools) {
@@ -277,7 +277,7 @@ virConnectListDefinedStoragePools(virConnectPtr conn,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (conn->storageDriver && conn->storageDriver->connectListDefinedStoragePools) {
@@ -1268,7 +1268,7 @@ virStoragePoolListVolumes(virStoragePoolPtr pool,
     virResetLastError();
 
     virCheckStoragePoolReturn(pool, -1);
-    virCheckNonNullArgGoto(names, error);
+    virCheckNonNullArrayArgGoto(names, maxnames, error);
     virCheckNonNegativeArgGoto(maxnames, error);
 
     if (pool->conn->storageDriver && pool->conn->storageDriver->storagePoolListVolumes) {