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>
This commit is contained in:
Erik Skultety 2019-11-18 12:04:16 +01:00
parent bf2988235c
commit d6064e2759
10 changed files with 33 additions and 29 deletions

View File

@ -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.
*

View File

@ -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) {

View File

@ -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(

View File

@ -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);

View File

@ -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) {

View File

@ -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) {

View File

@ -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) {

View File

@ -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) {

View File

@ -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) {

View File

@ -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) {