sys/newlib_syscalls_default: fix race condition in __sinit()

maribu · maribu · commit 375aed13e6d9 · 2024-02-16T07:21:28.000+01:00
This eagerly calls `__sinit()` instead of lazy initialization upon the first call to stdio (e.g. `puts()`, `printf()`). The issue is that without locking (as is currently the case for all RIOT platforms but ESP) two concurrent "first calls" may result in concurrent initialization of the same structure and data corruption. Fixes #20067
diff --git a/sys/newlib_syscalls_default/syscalls.c b/sys/newlib_syscalls_default/syscalls.c
@@ -24,32 +24,27 @@
  * @}
  */
 
-#include <unistd.h>
-#include <reent.h>
 #include <errno.h>
 #include <malloc.h>
+#include <reent.h>
+#include <stdint.h>
 #include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
 #include <sys/stat.h>
+#include <sys/times.h>
 #include <sys/unistd.h>
-#include <stdint.h>
+#include <unistd.h>
 
-#include "cpu.h"
-#include "board.h"
-#include "sched.h"
-#include "thread.h"
-#include "irq.h"
 #include "log.h"
+#include "modules.h"
 #include "periph/pm.h"
+#include "sched.h"
+#include "stdio_base.h"
+#include "thread.h"
+
 #if MODULE_VFS
 #include "vfs.h"
 #endif
 
-#include "stdio_base.h"
-
-#include <sys/times.h>
-
 #ifdef MODULE_XTIMER
 #include <sys/time.h>
 #include "div.h"
@@ -149,7 +144,23 @@ static const struct heap heaps[NUM_HEAPS] = {
  */
 void _init(void)
 {
-    /* nothing to do here */
+    /* Definition copied from newlib/libc/stdio/local.h */
+    extern void __sinit (struct _reent *);
+
+    /* When running multiple threads: Initialize reentrant structure before the
+     * scheduler starts. This normally happens upon the first stdio function
+     * called. However, if no boot message happens this can result in two
+     * concurrent "first calls" to stdio in data corruption, if no locking is
+     * used. Except for ESP (which is using its own syscalls.c anyway), this
+     * currently is the case in RIOT. */
+    if (MAXTHREADS > 1) {
+        /* Also, make an exception for riotboot, which does not use stdio
+         * at all. This would pull in stdio and increase .text size
+         * significantly there */
+        if (!IS_USED(MODULE_RIOTBOOT)) {
+            __sinit(_REENT);
+        }
+    }
 }
 
 /**
