1. Patchew
  2. Libvirt
  3. View series

[PATCH] qemu: validate: Validate maximum start time for <clock offset='absolute'>

Peter Krempa posted 1 patch 3 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/6d13488f6585561dd87691305b6ab0e411e971ce.1667488623.git.pkrempa@redhat.com
src/conf/schemas/domaincommon.rng               |  2 +-
src/qemu/qemu_validate.c                        | 17 +++++++++++++++++
.../clock-absolute.x86_64-latest.args           |  2 +-
tests/qemuxml2argvdata/clock-absolute.xml       |  2 +-
.../clock-absolute.x86_64-latest.xml            |  2 +-
5 files changed, 21 insertions(+), 4 deletions(-)
[PATCH] qemu: validate: Validate maximum start time for <clock offset='absolute'>
Posted by Peter Krempa 3 years, 3 months ago 
Glib can internally convert only unix timestamps up to
9999-12-31T23:59:59 (253402300799). Validate that the user doesn't use
more than that as otherwise we cause an assertion failure:

 (process:1183396): GLib-CRITICAL **: 14:25:00.906: g_date_time_format: assertion 'datetime != NULL' failed

Additionally adjust the schema to allow bigger values as we use
'unsigned long long' to parse the value.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2128993
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/schemas/domaincommon.rng               |  2 +-
 src/qemu/qemu_validate.c                        | 17 +++++++++++++++++
 .../clock-absolute.x86_64-latest.args           |  2 +-
 tests/qemuxml2argvdata/clock-absolute.xml       |  2 +-
 .../clock-absolute.x86_64-latest.xml            |  2 +-
 5 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index ebb39de3ef..cefe818044 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -1312,7 +1312,7 @@
               <value>absolute</value>
             </attribute>
             <attribute name="start">
-              <ref name="unsignedInt"/>
+              <ref name="unsignedLong"/>
             </attribute>
           </group>
         </choice>
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 1456a69351..1d4081e47e 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -663,6 +663,23 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def,
         }
     }

+    switch ((virDomainClockOffsetType) def->clock.offset) {
+    case VIR_DOMAIN_CLOCK_OFFSET_ABSOLUTE:
+        /* maximum timestamp glib can convert is 9999-12-31T23:59:59 */
+        if (def->clock.data.starttime > 253402300799) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("The maximum 'start' value for <clock offset='absolute'> is 253402300799"));
+            return -1;
+        }
+
+    case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+    case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+    case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
+    case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
+    case VIR_DOMAIN_CLOCK_OFFSET_LAST:
+        break;
+    }
+
     return 0;
 }

diff --git a/tests/qemuxml2argvdata/clock-absolute.x86_64-latest.args b/tests/qemuxml2argvdata/clock-absolute.x86_64-latest.args
index 75fc162cee..af64bc0f1a 100644
--- a/tests/qemuxml2argvdata/clock-absolute.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/clock-absolute.x86_64-latest.args
@@ -23,7 +23,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
 -nodefaults \
 -chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
 -mon chardev=charmonitor,id=monitor,mode=control \
--rtc base=1970-01-01T00:20:34 \
+-rtc base=9999-12-31T23:59:59 \
 -no-shutdown \
 -no-acpi \
 -boot strict=on \
diff --git a/tests/qemuxml2argvdata/clock-absolute.xml b/tests/qemuxml2argvdata/clock-absolute.xml
index e79f53ed3c..63ef7f6f33 100644
--- a/tests/qemuxml2argvdata/clock-absolute.xml
+++ b/tests/qemuxml2argvdata/clock-absolute.xml
@@ -8,7 +8,7 @@
     <type arch='x86_64' machine='pc'>hvm</type>
     <boot dev='hd'/>
   </os>
-  <clock offset='absolute' start='1234'/>
+  <clock offset='absolute' start='253402300799'/>
   <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>destroy</on_crash>
diff --git a/tests/qemuxml2xmloutdata/clock-absolute.x86_64-latest.xml b/tests/qemuxml2xmloutdata/clock-absolute.x86_64-latest.xml
index b313a74039..eea68677d5 100644
--- a/tests/qemuxml2xmloutdata/clock-absolute.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/clock-absolute.x86_64-latest.xml
@@ -11,7 +11,7 @@
   <cpu mode='custom' match='exact' check='none'>
     <model fallback='forbid'>qemu64</model>
   </cpu>
-  <clock offset='absolute' start='1234'/>
+  <clock offset='absolute' start='253402300799'/>
   <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>destroy</on_crash>
-- 
2.37.3
Re: [PATCH] qemu: validate: Validate maximum start time for <clock offset='absolute'>
Posted by Ján Tomko 3 years, 3 months ago 
On a Thursday in 2022, Peter Krempa wrote:
>Glib can internally convert only unix timestamps up to
>9999-12-31T23:59:59 (253402300799). Validate that the user doesn't use
>more than that as otherwise we cause an assertion failure:
>
> (process:1183396): GLib-CRITICAL **: 14:25:00.906: g_date_time_format: assertion 'datetime != NULL' failed
>
>Additionally adjust the schema to allow bigger values as we use
>'unsigned long long' to parse the value.
>
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2128993
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/conf/schemas/domaincommon.rng               |  2 +-
> src/qemu/qemu_validate.c                        | 17 +++++++++++++++++
> .../clock-absolute.x86_64-latest.args           |  2 +-
> tests/qemuxml2argvdata/clock-absolute.xml       |  2 +-
> .../clock-absolute.x86_64-latest.xml            |  2 +-
> 5 files changed, 21 insertions(+), 4 deletions(-)
>
>diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
>index ebb39de3ef..cefe818044 100644
>--- a/src/conf/schemas/domaincommon.rng
>+++ b/src/conf/schemas/domaincommon.rng
>@@ -1312,7 +1312,7 @@
>               <value>absolute</value>
>             </attribute>
>             <attribute name="start">
>-              <ref name="unsignedInt"/>
>+              <ref name="unsignedLong"/>
>             </attribute>
>           </group>
>         </choice>
>diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>index 1456a69351..1d4081e47e 100644
>--- a/src/qemu/qemu_validate.c
>+++ b/src/qemu/qemu_validate.c
>@@ -663,6 +663,23 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def,
>         }
>     }
>
>+    switch ((virDomainClockOffsetType) def->clock.offset) {
>+    case VIR_DOMAIN_CLOCK_OFFSET_ABSOLUTE:
>+        /* maximum timestamp glib can convert is 9999-12-31T23:59:59 */

Consider #defining this as a constant, e.g.

#define QEMU_MAX_GLIB_TIMESTAMP (2932896 * (24 * 60 * 60) + 23 * (60 * 60) + 59 * (60) + 59)

>+        if (def->clock.data.starttime > 253402300799) {
>+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>+                           _("The maximum 'start' value for <clock offset='absolute'> is 253402300799"));
>+            return -1;
>+        }
>+

Regardless of my above suggestion:

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.