[PATCH v2 1/1] remote: properly initialize objects in ACL helpers

Commit 2ecdf259299813c2c674377e22a0acbce5ccbbb2 was intended to implement two things: reduce stack usage inside ACL helpers and minimally initialize virDomainDef object to avoid passing garbage inside validation framework. Though original commit has not touched other ACL helpers. This patch adds proper clauses to remoteRelayNetworkEventCheckACL remoteRelayStoragePoolEventCheckACL remoteRelayNodeDeviceEventCheckACL remoteRelaySecretEventCheckACL Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Peter Krempa <pkrempa@redhat.com> CC: Roman Grigoriev <rgrigoriev@astralinux.ru> --- src/remote/remote_daemon_dispatch.c | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) Changes from v1: * g_autoptr is replaced with g_autofree upon reached consensus * patch 1 in series has been dropped diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index aaabd1e56c..b566a510b8 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -180,21 +180,21 @@ static bool remoteRelayNetworkEventCheckACL(virNetServerClient *client, virConnectPtr conn, virNetworkPtr net) { - virNetworkDef def; + g_autofree virNetworkDef *def = g_new0(virNetworkDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false; /* For now, we just create a virNetworkDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = net->name; - memcpy(def.uuid, net->uuid, VIR_UUID_BUFLEN); + def->name = net->name; + memcpy(def->uuid, net->uuid, VIR_UUID_BUFLEN); if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectNetworkEventRegisterAnyCheckACL(conn, &def); + ret = virConnectNetworkEventRegisterAnyCheckACL(conn, def); cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -206,21 +206,21 @@ remoteRelayStoragePoolEventCheckACL(virNetServerClient *client, virConnectPtr conn, virStoragePoolPtr pool) { - virStoragePoolDef def; + g_autofree virStoragePoolDef *def = g_new0(virStoragePoolDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false; /* For now, we just create a virStoragePoolDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = pool->name; - memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN); + def->name = pool->name; + memcpy(def->uuid, pool->uuid, VIR_UUID_BUFLEN); if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, &def); + ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, def); cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -232,20 +232,20 @@ remoteRelayNodeDeviceEventCheckACL(virNetServerClient *client, virConnectPtr conn, virNodeDevicePtr dev) { - virNodeDeviceDef def; + g_autofree virNodeDeviceDef *def = g_new0(virNodeDeviceDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false; /* For now, we just create a virNodeDeviceDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = dev->name; + def->name = dev->name; if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, &def); + ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, def); cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -257,22 +257,22 @@ remoteRelaySecretEventCheckACL(virNetServerClient *client, virConnectPtr conn, virSecretPtr secret) { - virSecretDef def; + g_autofree virSecretDef *def = g_new0(virSecretDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false; /* For now, we just create a virSecretDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - memcpy(def.uuid, secret->uuid, VIR_UUID_BUFLEN); - def.usage_type = secret->usageType; - def.usage_id = secret->usageID; + memcpy(def->uuid, secret->uuid, VIR_UUID_BUFLEN); + def->usage_type = secret->usageType; + def->usage_id = secret->usageID; if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectSecretEventRegisterAnyCheckACL(conn, &def); + ret = virConnectSecretEventRegisterAnyCheckACL(conn, def); cleanup: ignore_value(virIdentitySetCurrent(NULL)); -- 2.40.1

Commit 2ecdf259299813c2c674377e22a0acbce5ccbbb2 was intended to implement two things: reduce stack usage inside ACL helpers and minimally initialize virDomainDef object to avoid passing garbage inside validation framework. Though original commit has not touched other ACL helpers.
This patch adds proper clauses to remoteRelayNetworkEventCheckACL remoteRelayStoragePoolEventCheckACL remoteRelayNodeDeviceEventCheckACL remoteRelaySecretEventCheckACL
Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Peter Krempa <pkrempa@redhat.com> CC: Roman Grigoriev <rgrigoriev@astralinux.ru> --- src/remote/remote_daemon_dispatch.c | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-)
Changes from v1: * g_autoptr is replaced with g_autofree upon reached consensus * patch 1 in series has been dropped
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index aaabd1e56c..b566a510b8 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -180,21 +180,21 @@ static bool remoteRelayNetworkEventCheckACL(virNetServerClient *client, virConnectPtr conn, virNetworkPtr net) { - virNetworkDef def; + g_autofree virNetworkDef *def = g_new0(virNetworkDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false;
/* For now, we just create a virNetworkDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = net->name; - memcpy(def.uuid, net->uuid, VIR_UUID_BUFLEN); + def->name = net->name; + memcpy(def->uuid, net->uuid, VIR_UUID_BUFLEN);
if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectNetworkEventRegisterAnyCheckACL(conn, &def); + ret = virConnectNetworkEventRegisterAnyCheckACL(conn, def);
cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -206,21 +206,21 @@ remoteRelayStoragePoolEventCheckACL(virNetServerClient *client, virConnectPtr conn, virStoragePoolPtr pool) { - virStoragePoolDef def; + g_autofree virStoragePoolDef *def = g_new0(virStoragePoolDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false;
/* For now, we just create a virStoragePoolDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = pool->name; - memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN); + def->name = pool->name; + memcpy(def->uuid, pool->uuid, VIR_UUID_BUFLEN);
if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, &def); + ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, def);
cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -232,20 +232,20 @@ remoteRelayNodeDeviceEventCheckACL(virNetServerClient *client, virConnectPtr conn, virNodeDevicePtr dev) { - virNodeDeviceDef def; + g_autofree virNodeDeviceDef *def = g_new0(virNodeDeviceDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false;
/* For now, we just create a virNodeDeviceDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = dev->name; + def->name = dev->name;
if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, &def); + ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, def);
cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -257,22 +257,22 @@ remoteRelaySecretEventCheckACL(virNetServerClient *client, virConnectPtr conn, virSecretPtr secret) { - virSecretDef def; + g_autofree virSecretDef *def = g_new0(virSecretDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false;
/* For now, we just create a virSecretDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - memcpy(def.uuid, secret->uuid, VIR_UUID_BUFLEN); - def.usage_type = secret->usageType; - def.usage_id = secret->usageID; + memcpy(def->uuid, secret->uuid, VIR_UUID_BUFLEN); + def->usage_type = secret->usageType; + def->usage_id = secret->usageID;
if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectSecretEventRegisterAnyCheckACL(conn, &def); + ret = virConnectSecretEventRegisterAnyCheckACL(conn, def);
cleanup: ignore_value(virIdentitySetCurrent(NULL));
On 3/19/24 15:07, Denis V. Lunev wrote: ping

Commit 2ecdf259299813c2c674377e22a0acbce5ccbbb2 was intended to implement two things: reduce stack usage inside ACL helpers and minimally initialize virDomainDef object to avoid passing garbage inside validation framework. Though original commit has not touched other ACL helpers.
This patch adds proper clauses to remoteRelayNetworkEventCheckACL remoteRelayStoragePoolEventCheckACL remoteRelayNodeDeviceEventCheckACL remoteRelaySecretEventCheckACL
Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Peter Krempa <pkrempa@redhat.com> CC: Roman Grigoriev <rgrigoriev@astralinux.ru> --- src/remote/remote_daemon_dispatch.c | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-)
Changes from v1: * g_autoptr is replaced with g_autofree upon reached consensus * patch 1 in series has been dropped
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index aaabd1e56c..b566a510b8 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -180,21 +180,21 @@ static bool remoteRelayNetworkEventCheckACL(virNetServerClient *client, virConnectPtr conn, virNetworkPtr net) { - virNetworkDef def; + g_autofree virNetworkDef *def = g_new0(virNetworkDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false;
/* For now, we just create a virNetworkDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = net->name; - memcpy(def.uuid, net->uuid, VIR_UUID_BUFLEN); + def->name = net->name; + memcpy(def->uuid, net->uuid, VIR_UUID_BUFLEN);
if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectNetworkEventRegisterAnyCheckACL(conn, &def); + ret = virConnectNetworkEventRegisterAnyCheckACL(conn, def);
cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -206,21 +206,21 @@ remoteRelayStoragePoolEventCheckACL(virNetServerClient *client, virConnectPtr conn, virStoragePoolPtr pool) { - virStoragePoolDef def; + g_autofree virStoragePoolDef *def = g_new0(virStoragePoolDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false;
/* For now, we just create a virStoragePoolDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = pool->name; - memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN); + def->name = pool->name; + memcpy(def->uuid, pool->uuid, VIR_UUID_BUFLEN);
if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, &def); + ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, def);
cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -232,20 +232,20 @@ remoteRelayNodeDeviceEventCheckACL(virNetServerClient *client, virConnectPtr conn, virNodeDevicePtr dev) { - virNodeDeviceDef def; + g_autofree virNodeDeviceDef *def = g_new0(virNodeDeviceDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false;
/* For now, we just create a virNodeDeviceDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = dev->name; + def->name = dev->name;
if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, &def); + ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, def);
cleanup: ignore_value(virIdentitySetCurrent(NULL)); @@ -257,22 +257,22 @@ remoteRelaySecretEventCheckACL(virNetServerClient *client, virConnectPtr conn, virSecretPtr secret) { - virSecretDef def; + g_autofree virSecretDef *def = g_new0(virSecretDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false;
/* For now, we just create a virSecretDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - memcpy(def.uuid, secret->uuid, VIR_UUID_BUFLEN); - def.usage_type = secret->usageType; - def.usage_id = secret->usageID; + memcpy(def->uuid, secret->uuid, VIR_UUID_BUFLEN); + def->usage_type = secret->usageType; + def->usage_id = secret->usageID;
if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectSecretEventRegisterAnyCheckACL(conn, &def); + ret = virConnectSecretEventRegisterAnyCheckACL(conn, def);
cleanup: ignore_value(virIdentitySetCurrent(NULL));
On 3/19/24 15:07, Denis V. Lunev wrote: ping v2

On Tue, Mar 19, 2024 at 15:07:04 +0100, Denis V. Lunev wrote:
Commit 2ecdf259299813c2c674377e22a0acbce5ccbbb2 was intended to implement two things: reduce stack usage inside ACL helpers and minimally initialize virDomainDef object to avoid passing garbage inside validation framework. Though original commit has not touched other ACL helpers.
This patch adds proper clauses to remoteRelayNetworkEventCheckACL remoteRelayStoragePoolEventCheckACL remoteRelayNodeDeviceEventCheckACL remoteRelaySecretEventCheckACL
Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Peter Krempa <pkrempa@redhat.com> CC: Roman Grigoriev <rgrigoriev@astralinux.ru>
I'll drop the CC lines if you are okay with that as it doesn't seem to be needed to record that info in git.
--- src/remote/remote_daemon_dispatch.c | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-)
Changes from v1: * g_autoptr is replaced with g_autofree upon reached consensus * patch 1 in series has been dropped
Reviewed-by: Peter Krempa <pkrempa@redhat.com> and I'll push it after the release is done later today.

On 4/2/24 09:31, Peter Krempa wrote:
On Tue, Mar 19, 2024 at 15:07:04 +0100, Denis V. Lunev wrote:
Commit 2ecdf259299813c2c674377e22a0acbce5ccbbb2 was intended to implement two things: reduce stack usage inside ACL helpers and minimally initialize virDomainDef object to avoid passing garbage inside validation framework. Though original commit has not touched other ACL helpers.
This patch adds proper clauses to remoteRelayNetworkEventCheckACL remoteRelayStoragePoolEventCheckACL remoteRelayNodeDeviceEventCheckACL remoteRelaySecretEventCheckACL
Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Peter Krempa <pkrempa@redhat.com> CC: Roman Grigoriev <rgrigoriev@astralinux.ru> I'll drop the CC lines if you are okay with that as it doesn't seem to be needed to record that info in git.
--- src/remote/remote_daemon_dispatch.c | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-)
Changes from v1: * g_autoptr is replaced with g_autofree upon reached consensus * patch 1 in series has been dropped
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
and I'll push it after the release is done later today.
No prob at all, thanks!
participants (3)
-
Denis V. Lunev
-
Denis V. Lunev
-
Peter Krempa