[PATCH] error-report: fix crash when compute iso8061 time

[PATCH] error-report: fix crash when compute iso8061 time

[Lei He]

g_get_real_time() returns the number of MICROSECONDS since
January 1, 1970 UTC, but g_date_time_new_from_unix_utc() expects
a timestamp in SECONDS.

Directly call g_data_time_new_from_unix_utc(g_get_real_time()) causes
overflow and a NULL pointer is returned, then qemu crashes.

Use g_date_time_new_now_utc() instead, and add a check for NULL result.

Signed-off-by: Lei He <helei.sig11@bytedance.com>
---
 util/error-report.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/util/error-report.c b/util/error-report.c
index dbadaf206d..d3c150661d 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -173,10 +173,13 @@ static char *
 real_time_iso8601(void)
 {
 #if GLIB_CHECK_VERSION(2,62,0)
-    g_autoptr(GDateTime) dt = g_date_time_new_from_unix_utc(g_get_real_time());
+    g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
     /* ignore deprecation warning, since GLIB_VERSION_MAX_ALLOWED is 2.56 */
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+    if (!dt) {
+        return NULL;
+    }
     return g_date_time_format_iso8601(dt);
 #pragma GCC diagnostic pop
 #else
@@ -199,8 +202,10 @@ static void vreport(report_type type, const char *fmt, va_list ap)
 
     if (message_with_timestamp && !monitor_cur()) {
         timestr = real_time_iso8601();
-        error_printf("%s ", timestr);
-        g_free(timestr);
+        if (timestr) {
+            error_printf("%s ", timestr);
+            g_free(timestr);
+        }
     }
 
     /* Only prepend guest name if -msg guest-name and -name guest=... are set */
-- 
2.11.0

Re: [PATCH] error-report: fix crash when compute iso8061 time

[Marc-André Lureau]

Hi

On Thu, Apr 28, 2022 at 4:15 AM Lei He <helei.sig11@bytedance.com> wrote:
>
> g_get_real_time() returns the number of MICROSECONDS since
> January 1, 1970 UTC, but g_date_time_new_from_unix_utc() expects
> a timestamp in SECONDS.
>
> Directly call g_data_time_new_from_unix_utc(g_get_real_time()) causes
> overflow and a NULL pointer is returned, then qemu crashes.
>
> Use g_date_time_new_now_utc() instead, and add a check for NULL result.
>
> Signed-off-by: Lei He <helei.sig11@bytedance.com>

A fix is already in Paolo last pull request:
https://patchew.org/QEMU/20220428065335.189795-1-pbonzini@redhat.com/20220428065335.189795-2-pbonzini@redhat.com/

thanks

> ---
>  util/error-report.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/util/error-report.c b/util/error-report.c
> index dbadaf206d..d3c150661d 100644
> --- a/util/error-report.c
> +++ b/util/error-report.c
> @@ -173,10 +173,13 @@ static char *
>  real_time_iso8601(void)
>  {
>  #if GLIB_CHECK_VERSION(2,62,0)
> -    g_autoptr(GDateTime) dt = g_date_time_new_from_unix_utc(g_get_real_time());
> +    g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
>      /* ignore deprecation warning, since GLIB_VERSION_MAX_ALLOWED is 2.56 */
>  #pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +    if (!dt) {
> +        return NULL;
> +    }
>      return g_date_time_format_iso8601(dt);
>  #pragma GCC diagnostic pop
>  #else
> @@ -199,8 +202,10 @@ static void vreport(report_type type, const char *fmt, va_list ap)
>
>      if (message_with_timestamp && !monitor_cur()) {
>          timestr = real_time_iso8601();
> -        error_printf("%s ", timestr);
> -        g_free(timestr);
> +        if (timestr) {
> +            error_printf("%s ", timestr);
> +            g_free(timestr);
> +        }
>      }
>
>      /* Only prepend guest name if -msg guest-name and -name guest=... are set */
> --
> 2.11.0
>

Re: [External] [PATCH] error-report: fix crash when compute iso8061 time

[何磊]

Fine, just ignore this patch.

> On Apr 28, 2022, at 4:58 PM, Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> 
> Hi
> 
> On Thu, Apr 28, 2022 at 4:15 AM Lei He <helei.sig11@bytedance.com> wrote:
>> 
>> g_get_real_time() returns the number of MICROSECONDS since
>> January 1, 1970 UTC, but g_date_time_new_from_unix_utc() expects
>> a timestamp in SECONDS.
>> 
>> Directly call g_data_time_new_from_unix_utc(g_get_real_time()) causes
>> overflow and a NULL pointer is returned, then qemu crashes.
>> 
>> Use g_date_time_new_now_utc() instead, and add a check for NULL result.
>> 
>> Signed-off-by: Lei He <helei.sig11@bytedance.com>
> 
> A fix is already in Paolo last pull request:
> https://patchew.org/QEMU/20220428065335.189795-1-pbonzini@redhat.com/20220428065335.189795-2-pbonzini@redhat.com/
> 
> thanks
> 
>> ---
>> util/error-report.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>> 
>> diff --git a/util/error-report.c b/util/error-report.c
>> index dbadaf206d..d3c150661d 100644
>> --- a/util/error-report.c
>> +++ b/util/error-report.c
>> @@ -173,10 +173,13 @@ static char *
>> real_time_iso8601(void)
>> {
>> #if GLIB_CHECK_VERSION(2,62,0)
>> -    g_autoptr(GDateTime) dt = g_date_time_new_from_unix_utc(g_get_real_time());
>> +    g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
>>     /* ignore deprecation warning, since GLIB_VERSION_MAX_ALLOWED is 2.56 */
>> #pragma GCC diagnostic push
>> #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>> +    if (!dt) {
>> +        return NULL;
>> +    }
>>     return g_date_time_format_iso8601(dt);
>> #pragma GCC diagnostic pop
>> #else
>> @@ -199,8 +202,10 @@ static void vreport(report_type type, const char *fmt, va_list ap)
>> 
>>     if (message_with_timestamp && !monitor_cur()) {
>>         timestr = real_time_iso8601();
>> -        error_printf("%s ", timestr);
>> -        g_free(timestr);
>> +        if (timestr) {
>> +            error_printf("%s ", timestr);
>> +            g_free(timestr);
>> +        }
>>     }
>> 
>>     /* Only prepend guest name if -msg guest-name and -name guest=... are set */
>> --
>> 2.11.0
>> 
>