Some monitor functions, most notably, monitor_cur() rely on global
data being initialized by 'monitor_init_globals()'. The latter is
called relatively late in startup. If code triggers error_report()
before monitor_init_globals() is called, QEMU will abort when
accessing the uninitialized monitor mutex.

The critical monitor global data must be initialized from a
constructor function, to improve the guarantee that it is done
before any possible calls to monitor_cur(). Not only that, but
the constructor must be marked to run before the default
constructor in case any of them trigger error reporting.

Note in particular that the RCU constructor will spawn a background
thread so we might even have non-constructor QEMU code running
concurrently with other constructors.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/monitor.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/monitor/monitor.c b/monitor/monitor.c
index c5a5d30877..da54e1b1ce 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -704,18 +704,22 @@ void monitor_cleanup(void)
     }
 }
 
-static void monitor_qapi_event_init(void)
+/*
+ * Initialize static vars that have no deps on external
+ * module initialization, and are required for external
+ * functions to call things like monitor_cur()
+ */
+static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
+monitor_init_static(void)
 {
+    qemu_mutex_init(&monitor_lock);
+    coroutine_mon = g_hash_table_new(NULL, NULL);
     monitor_qapi_event_state = g_hash_table_new(qapi_event_throttle_hash,
                                                 qapi_event_throttle_equal);
 }
 
 void monitor_init_globals(void)
 {
-    monitor_qapi_event_init();
-    qemu_mutex_init(&monitor_lock);
-    coroutine_mon = g_hash_table_new(NULL, NULL);
-
     /*
      * The dispatcher BH must run in the main loop thread, since we
      * have commands assuming that context.  It would be nice to get
-- 
2.50.1

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> Some monitor functions, most notably, monitor_cur() rely on global
> data being initialized by 'monitor_init_globals()'. The latter is
> called relatively late in startup. If code triggers error_report()
> before monitor_init_globals() is called, QEMU will abort when
> accessing the uninitialized monitor mutex.
> 
> The critical monitor global data must be initialized from a
> constructor function, to improve the guarantee that it is done
> before any possible calls to monitor_cur(). Not only that, but
> the constructor must be marked to run before the default
> constructor in case any of them trigger error reporting.
> 
> Note in particular that the RCU constructor will spawn a background
> thread so we might even have non-constructor QEMU code running
> concurrently with other constructors.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Checked that qemu_mutex_init doesn't look like it can call
error code; so

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

> ---
>  monitor/monitor.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index c5a5d30877..da54e1b1ce 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -704,18 +704,22 @@ void monitor_cleanup(void)
>      }
>  }
>  
> -static void monitor_qapi_event_init(void)
> +/*
> + * Initialize static vars that have no deps on external
> + * module initialization, and are required for external
> + * functions to call things like monitor_cur()
> + */
> +static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
> +monitor_init_static(void)
>  {
> +    qemu_mutex_init(&monitor_lock);
> +    coroutine_mon = g_hash_table_new(NULL, NULL);
>      monitor_qapi_event_state = g_hash_table_new(qapi_event_throttle_hash,
>                                                  qapi_event_throttle_equal);
>  }
>  
>  void monitor_init_globals(void)
>  {
> -    monitor_qapi_event_init();
> -    qemu_mutex_init(&monitor_lock);
> -    coroutine_mon = g_hash_table_new(NULL, NULL);
> -
>      /*
>       * The dispatcher BH must run in the main loop thread, since we
>       * have commands assuming that context.  It would be nice to get
> -- 
> 2.50.1
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

On 8/30/25 04:03, Daniel P. Berrangé wrote:
> Some monitor functions, most notably, monitor_cur() rely on global
> data being initialized by 'monitor_init_globals()'. The latter is
> called relatively late in startup. If code triggers error_report()
> before monitor_init_globals() is called, QEMU will abort when
> accessing the uninitialized monitor mutex.
> 
> The critical monitor global data must be initialized from a
> constructor function, to improve the guarantee that it is done
> before any possible calls to monitor_cur(). Not only that, but
> the constructor must be marked to run before the default
> constructor in case any of them trigger error reporting.
> 
> Note in particular that the RCU constructor will spawn a background
> thread so we might even have non-constructor QEMU code running
> concurrently with other constructors.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   monitor/monitor.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~