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(-)
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.
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
On a Monday in 2020, 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. > > 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(-) > Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
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
>
© 2016 - 2024 Red Hat, Inc.