[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt PATCH] virgdbus: add DBus reply format check



On Mon, Sep 21, 2020 at 03:49:34PM +0200, Pavel Hrdina wrote:
> We used to check the format of reply data with libdbus so we should do
> the same with GLib DBus as well.
> 
> Signed-off-by: Pavel Hrdina <phrdina redhat com>
> ---
> 
> At first I thought that this is not necessary as it is unlikely to
> happen but after Jano found the bug with firewalld getZones function
> and asked about checking return values I figured out that it will be
> better to check it because if the returned message would have different
> format it would be silently ignored.

I know it's to soon for ping but we should get this in before release.

>  src/rpc/virnetdaemon.c  | 1 +
>  src/util/virfirewalld.c | 5 +++++
>  src/util/virgdbus.c     | 8 ++++++--
>  src/util/virgdbus.h     | 2 ++
>  src/util/virpolkit.c    | 1 +
>  src/util/virsystemd.c   | 7 +++++++
>  6 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
> index 12d4d9bf87..f3a5e9f75c 100644
> --- a/src/rpc/virnetdaemon.c
> +++ b/src/rpc/virnetdaemon.c
> @@ -487,6 +487,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
>  
>      rc = virGDBusCallMethodWithFD(systemBus,
>                                    &reply,
> +                                  G_VARIANT_TYPE("(h)"),
>                                    &replyFD,
>                                    NULL,
>                                    "org.freedesktop.login1",
> diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
> index 12448f0681..a94ac7c183 100644
> --- a/src/util/virfirewalld.c
> +++ b/src/util/virfirewalld.c
> @@ -95,6 +95,7 @@ virFirewallDGetVersion(unsigned long *version)
>  
>      if (virGDBusCallMethod(sysbus,
>                             &reply,
> +                           G_VARIANT_TYPE("(v)"),
>                             NULL,
>                             VIR_FIREWALL_FIREWALLD_SERVICE,
>                             "/org/fedoraproject/FirewallD1",
> @@ -147,6 +148,7 @@ virFirewallDGetBackend(void)
>  
>      if (virGDBusCallMethod(sysbus,
>                             &reply,
> +                           G_VARIANT_TYPE("(v)"),
>                             error,
>                             VIR_FIREWALL_FIREWALLD_SERVICE,
>                             "/org/fedoraproject/FirewallD1/config",
> @@ -207,6 +209,7 @@ virFirewallDGetZones(char ***zones, size_t *nzones)
>  
>      if (virGDBusCallMethod(sysbus,
>                             &reply,
> +                           G_VARIANT_TYPE("(as)"),
>                             NULL,
>                             VIR_FIREWALL_FIREWALLD_SERVICE,
>                             "/org/fedoraproject/FirewallD1",
> @@ -295,6 +298,7 @@ virFirewallDApplyRule(virFirewallLayer layer,
>  
>      if (virGDBusCallMethod(sysbus,
>                             &reply,
> +                           G_VARIANT_TYPE("(s)"),
>                             error,
>                             VIR_FIREWALL_FIREWALLD_SERVICE,
>                             "/org/fedoraproject/FirewallD1",
> @@ -357,6 +361,7 @@ virFirewallDInterfaceSetZone(const char *iface,
>      message = g_variant_new("(ss)", zone, iface);
>  
>      return virGDBusCallMethod(sysbus,
> +                             NULL,
>                               NULL,
>                               NULL,
>                               VIR_FIREWALL_FIREWALLD_SERVICE,
> diff --git a/src/util/virgdbus.c b/src/util/virgdbus.c
> index 535b19f0a4..837c8faf1f 100644
> --- a/src/util/virgdbus.c
> +++ b/src/util/virgdbus.c
> @@ -181,6 +181,7 @@ virGDBusCloseSystemBus(void)
>   * virGDBusCallMethod:
>   * @conn: a DBus connection
>   * @reply: pointer to receive reply message, or NULL
> + * @replyType: pointer to GVariantType to validate reply data, or NULL
>   * @error: libvirt error pointer or NULL
>   * @busName: bus identifier of the target service
>   * @objectPath: object path of the target service
> @@ -198,6 +199,7 @@ virGDBusCloseSystemBus(void)
>  int
>  virGDBusCallMethod(GDBusConnection *conn,
>                     GVariant **reply,
> +                   const GVariantType *replyType,
>                     virErrorPtr error,
>                     const char *busName,
>                     const char *objectPath,
> @@ -220,7 +222,7 @@ virGDBusCallMethod(GDBusConnection *conn,
>                                        ifaceName,
>                                        method,
>                                        data,
> -                                      NULL,
> +                                      replyType,
>                                        G_DBUS_CALL_FLAGS_NONE,
>                                        VIR_DBUS_METHOD_CALL_TIMEOUT_MILIS,
>                                        NULL,
> @@ -250,6 +252,7 @@ virGDBusCallMethod(GDBusConnection *conn,
>  int
>  virGDBusCallMethodWithFD(GDBusConnection *conn,
>                           GVariant **reply,
> +                         const GVariantType *replyType,
>                           GUnixFDList **replyFD,
>                           virErrorPtr error,
>                           const char *busName,
> @@ -274,7 +277,7 @@ virGDBusCallMethodWithFD(GDBusConnection *conn,
>                                                          ifaceName,
>                                                          method,
>                                                          data,
> -                                                        NULL,
> +                                                        replyType,
>                                                          G_DBUS_CALL_FLAGS_NONE,
>                                                          VIR_DBUS_METHOD_CALL_TIMEOUT_MILIS,
>                                                          dataFD,
> @@ -342,6 +345,7 @@ virGDBusIsServiceInList(const char *listMethod,
>  
>      rc = virGDBusCallMethod(conn,
>                              &reply,
> +                            G_VARIANT_TYPE("(as)"),
>                              NULL,
>                              "org.freedesktop.DBus",
>                              "/org/freedesktop/DBus",
> diff --git a/src/util/virgdbus.h b/src/util/virgdbus.h
> index 6ea717eea2..ca7073e27c 100644
> --- a/src/util/virgdbus.h
> +++ b/src/util/virgdbus.h
> @@ -45,6 +45,7 @@ virGDBusCloseSystemBus(void);
>  int
>  virGDBusCallMethod(GDBusConnection *conn,
>                     GVariant **reply,
> +                   const GVariantType *replyType,
>                     virErrorPtr error,
>                     const char *busName,
>                     const char *objectPath,
> @@ -55,6 +56,7 @@ virGDBusCallMethod(GDBusConnection *conn,
>  int
>  virGDBusCallMethodWithFD(GDBusConnection *conn,
>                           GVariant **reply,
> +                         const GVariantType *replyType,
>                           GUnixFDList **replyFD,
>                           virErrorPtr error,
>                           const char *busName,
> diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
> index 2ad00fd206..aad924a065 100644
> --- a/src/util/virpolkit.c
> +++ b/src/util/virpolkit.c
> @@ -104,6 +104,7 @@ int virPolkitCheckAuth(const char *actionid,
>  
>      if (virGDBusCallMethod(sysbus,
>                             &reply,
> +                           G_VARIANT_TYPE("((bba{ss}))"),
>                             NULL,
>                             "org.freedesktop.PolicyKit1",
>                             "/org/freedesktop/PolicyKit1/Authority",
> diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
> index 32c830c002..8456085476 100644
> --- a/src/util/virsystemd.c
> +++ b/src/util/virsystemd.c
> @@ -214,6 +214,7 @@ virSystemdGetMachineNameByPID(pid_t pid)
>  
>      if (virGDBusCallMethod(conn,
>                             &reply,
> +                           G_VARIANT_TYPE("(o)"),
>                             NULL,
>                             "org.freedesktop.machine1",
>                             "/org/freedesktop/machine1",
> @@ -236,6 +237,7 @@ virSystemdGetMachineNameByPID(pid_t pid)
>  
>      if (virGDBusCallMethod(conn,
>                             &reply,
> +                           G_VARIANT_TYPE("(v)"),
>                             NULL,
>                             "org.freedesktop.machine1",
>                             object,
> @@ -384,6 +386,7 @@ int virSystemdCreateMachine(const char *name,
>                                  gprops);
>  
>          rc = virGDBusCallMethod(conn,
> +                                NULL,
>                                  NULL,
>                                  error,
>                                  "org.freedesktop.machine1",
> @@ -430,6 +433,7 @@ int virSystemdCreateMachine(const char *name,
>                                  gprops);
>  
>          rc = virGDBusCallMethod(conn,
> +                                NULL,
>                                  NULL,
>                                  NULL,
>                                  "org.freedesktop.machine1",
> @@ -457,6 +461,7 @@ int virSystemdCreateMachine(const char *name,
>                                  gprops);
>  
>          rc = virGDBusCallMethod(conn,
> +                                NULL,
>                                  NULL,
>                                  NULL,
>                                  "org.freedesktop.systemd1",
> @@ -507,6 +512,7 @@ int virSystemdTerminateMachine(const char *name)
>  
>      VIR_DEBUG("Attempting to terminate machine via systemd");
>      if (virGDBusCallMethod(conn,
> +                           NULL,
>                             NULL,
>                             error,
>                             "org.freedesktop.machine1",
> @@ -592,6 +598,7 @@ virSystemdPMSupportTarget(const char *methodName, bool *result)
>  
>      if (virGDBusCallMethod(conn,
>                             &reply,
> +                           G_VARIANT_TYPE("(s)"),
>                             NULL,
>                             "org.freedesktop.login1",
>                             "/org/freedesktop/login1",
> -- 
> 2.26.2
> 

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]