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

["Denis V. Lunev" <den@xxxxxxxxxxxxx>]

On 3/19/24 15:07, 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@xxxxxxxxxx>
> CC: Peter Krempa <pkrempa@xxxxxxxxxx>
> CC: Roman Grigoriev <rgrigoriev@xxxxxxxxxxxxx>
> ---
>   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));
ping v2
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx