fix(esp_stdio): add fd-table locking, per-fd flag checks, and Kconfig cleanup

components/esp_stdio/Kconfig

@@ -185,23 +185,28 @@ menu "ESP-STDIO"
             If enabled, esp_rom_printf and ESP_EARLY_LOG output will also be sent over USB CDC.
             Disabling this option saves about 1kB or RAM.
 
+    config ESP_STDIO_BASIC_MODE
+        bool "Use basic /dev/console descriptor handling"
+        depends on VFS_SUPPORT_IO
+        default n
+        help
+            Enable the low-overhead legacy /dev/console descriptor handling.
+
+            In basic mode, all open() calls share a single internal descriptor. This keeps RAM usage
+            lower, but disables checks for per-fd behaviour, such as read-only versus write-only access.
+
     config ESP_STDIO_MAX_FDS
         int "Max logical stdio file descriptors"
-        depends on VFS_SUPPORT_IO
-        range 0 64
+        depends on VFS_SUPPORT_IO && !ESP_STDIO_BASIC_MODE
+        range 3 64
         default 16
         help
             Maximum number of logical /dev/console descriptors tracked by esp_stdio VFS.
-                - 0 enables basic mode with a shared internal descriptor.
-                - 1..2 is not allowed
-                - Values >= 3 select descriptor-table mode (minimum table size is 3).
 
             stdin, stdout, and stderr each occupy one slot. The default leaves room for additional
             opens of /dev/console (e.g. dup2, reopen). If set to exactly 3, only the three standard
             streams fit and further opens fail with ENFILE unless one is closed first.
 
-            Enabling basic mode decreases RAM usage, but disables checks for per-fd behaviour, such as read-only.
-
     config ESP_STDIO_MAX_VFS_ENTRIES
         int "Maximum number of registerable VFS sinks"
         depends on VFS_SUPPORT_IO

components/esp_stdio/stdio_vfs.c

@@ -36,6 +36,7 @@
 #include "esp_private/startup_internal.h"
 #include "esp_private/nullfs.h"
 #include "esp_heap_caps.h"
+#include <sys/lock.h>
 #endif
 
 #define STRINGIFY(s) STRINGIFY2(s)
@@ -51,9 +52,11 @@
 
 #if CONFIG_VFS_SUPPORT_IO
 
-_Static_assert(CONFIG_ESP_STDIO_MAX_FDS == 0 || CONFIG_ESP_STDIO_MAX_FDS >= 3, "Invalid value for CONFIG_ESP_STDIO_MAX_FDS");
+#if !CONFIG_ESP_STDIO_BASIC_MODE
+_Static_assert(CONFIG_ESP_STDIO_MAX_FDS >= 3, "Invalid value for CONFIG_ESP_STDIO_MAX_FDS");
+#endif
 
-#define ESP_STDIO_IS_BASIC (CONFIG_ESP_STDIO_MAX_FDS <= 0)
+#define ESP_STDIO_IS_BASIC (CONFIG_ESP_STDIO_BASIC_MODE)
 
 typedef struct {
     const esp_vfs_fs_ops_t *ops;
@@ -80,10 +83,11 @@ typedef struct {
 } context_t;
 
 static context_t s_ctx = {0};
+static _lock_t s_ctx_lock;
 
 static const char *TAG = "esp_stdio";
 
-static inline bool is_fd_valid(int fd)
+static inline bool is_fd_valid_nolock(int fd)
 {
 #if ESP_STDIO_IS_BASIC
     return (fd >= 0) && (s_ctx.fd_count > 0);
@@ -92,6 +96,37 @@ static inline bool is_fd_valid(int fd)
 #endif
 }
 
+static inline bool is_fd_valid(int fd)
+{
+    bool valid;
+
+    _lock_acquire(&s_ctx_lock);
+    valid = is_fd_valid_nolock(fd);
+    _lock_release(&s_ctx_lock);
+
+    return valid;
+}
+
+#if !ESP_STDIO_IS_BASIC
+static inline int get_fd_flags_nolock(int fd)
+{
+    assert((fd >= 0) && (fd < CONFIG_ESP_STDIO_MAX_FDS));
+    return s_ctx.fds[fd].flags;
+}
+
+static inline bool fd_allows_read_nolock(int fd)
+{
+    const int flags = get_fd_flags_nolock(fd);
+    return (flags & O_ACCMODE) != O_WRONLY;
+}
+
+static inline bool fd_allows_write_nolock(int fd)
+{
+    const int flags = get_fd_flags_nolock(fd);
+    return (flags & O_ACCMODE) != O_RDONLY;
+}
+#endif
+
 static inline mux_entry_t *get_primary_entry(void)
 {
     assert(s_ctx.used > 0);
@@ -169,15 +204,26 @@ int console_open(__attribute__((unused)) void *ctx, const char * path, int flags
 {
 #if ESP_STDIO_IS_BASIC
     (void) path;
+    (void) flags;
+    (void) mode;
+
+    _lock_acquire(&s_ctx_lock);
     s_ctx.fd_count++;
+    _lock_release(&s_ctx_lock);
+
     return 0;
 #else
+    (void) mode;
+
     if (!path || strcmp(path, "/") != 0) {
         errno = ENOENT;
         return -1;
     }
 
+    _lock_acquire(&s_ctx_lock);
+
     if (s_ctx.fd_count >= CONFIG_ESP_STDIO_MAX_FDS) {
+        _lock_release(&s_ctx_lock);
         errno = ENFILE;
         return -1;
     }
@@ -191,24 +237,28 @@ int console_open(__attribute__((unused)) void *ctx, const char * path, int flags
     }
 
     if (fd < 0) {
+        _lock_release(&s_ctx_lock);
         errno = ENFILE;
         return -1;
     }
 
     s_ctx.fd_count++;
-
     s_ctx.fds[fd] = (fd_entry_t) {
         .in_use = true,
         .flags = flags,
     };
 
+    _lock_release(&s_ctx_lock);
     return fd;
 #endif
 }
 
 int console_close(__attribute__((unused)) void *ctx, int fd)
 {
-    if (!is_fd_valid(fd)) {
+    _lock_acquire(&s_ctx_lock);
+
+    if (!is_fd_valid_nolock(fd)) {
+        _lock_release(&s_ctx_lock);
         errno = EBADF;
         return -1;
     }
@@ -220,23 +270,43 @@ int console_close(__attribute__((unused)) void *ctx, int fd)
     s_ctx.fd_count--;
 #endif
 
+    _lock_release(&s_ctx_lock);
     return 0;
 }
 
 ssize_t console_write(__attribute__((unused)) void *ctx, int fd, const void *data, size_t size)
 {
+#if ESP_STDIO_IS_BASIC
     if (!is_fd_valid(fd)) {
         errno = EBADF;
         return -1;
     }
-    for (size_t i = 0; i < s_ctx.used; i++) {
+#else
+    _lock_acquire(&s_ctx_lock);
+    if (!is_fd_valid_nolock(fd)) {
+        _lock_release(&s_ctx_lock);
+        errno = EBADF;
+        return -1;
+    }
+    if (!fd_allows_write_nolock(fd)) {
+        _lock_release(&s_ctx_lock);
+        errno = EBADF;
+        return -1;
+    }
+    _lock_release(&s_ctx_lock);
+#endif
+
+    const mux_entry_t *primary = get_primary_entry();
+    ssize_t ret_val = primary->ops->write_p(primary->vfs_ctx, primary->fd, data, size);
+
+    for (size_t i = 1; i < s_ctx.used; i++) {
         const mux_entry_t *entry = s_ctx.entries + i;
         if (entry->ops->write_p && entry->fd >= 0) {
-            entry->ops->write_p(entry->vfs_ctx, entry->fd, data, size);
+            (void) entry->ops->write_p(entry->vfs_ctx, entry->fd, data, size);
         }
     }
 
-    return size;
+    return ret_val;
 }
 
 int console_fstat(__attribute__((unused)) void *ctx, int fd, struct stat * st)
@@ -256,10 +326,25 @@ int console_fstat(__attribute__((unused)) void *ctx, int fd, struct stat * st)
 
 ssize_t console_read(__attribute__((unused)) void *ctx, int fd, void * dst, size_t size)
 {
+#if ESP_STDIO_IS_BASIC
     if (!is_fd_valid(fd)) {
         errno = EBADF;
         return -1;
     }
+#else
+    _lock_acquire(&s_ctx_lock);
+    if (!is_fd_valid_nolock(fd)) {
+        _lock_release(&s_ctx_lock);
+        errno = EBADF;
+        return -1;
+    }
+    if (!fd_allows_read_nolock(fd)) {
+        _lock_release(&s_ctx_lock);
+        errno = EBADF;
+        return -1;
+    }
+    _lock_release(&s_ctx_lock);
+#endif
 
     const mux_entry_t *entry = get_primary_entry();
     if (!entry->ops->read_p) {
@@ -302,7 +387,7 @@ int console_fsync(__attribute__((unused)) void *ctx, int fd)
     for (size_t i = 1; i < s_ctx.used; i++) {
         const mux_entry_t *entry = s_ctx.entries + i;
         if (entry->ops->fsync_p && entry->fd >= 0) {
-            entry->ops->fsync_p(entry->vfs_ctx, entry->fd);
+            (void) entry->ops->fsync_p(entry->vfs_ctx, entry->fd);
         }
     }
 
@@ -325,13 +410,8 @@ int console_access(__attribute__((unused)) void *ctx, const char *path, int amod
 
 #ifdef CONFIG_VFS_SUPPORT_SELECT
 
-/*
- * Logical /dev/console fds (0,1,2,...) are not UART port numbers. uart_vfs select uses fd_set bits as
- * SOC UART indices; without remapping, monitoring stdout/stderr selects UART1/UART2 and corrupts
- * s_uart_select_count / ISR registration (only UART0 is the console sink).
- */
 typedef struct {
-    void *uart_args;
+    void *sink_select_args;
     fd_set *readfds;
     fd_set *writefds;
     fd_set *exceptfds;
@@ -406,7 +486,7 @@ static esp_err_t console_start_select(int nfds, fd_set *readfds, fd_set *writefd
         return ESP_ERR_NO_MEM;
     }
 
-    ctx->uart_args = NULL;
+    ctx->sink_select_args = NULL;
     ctx->readfds = readfds;
     ctx->writefds = writefds;
     ctx->exceptfds = exceptfds;
@@ -415,7 +495,7 @@ static esp_err_t console_start_select(int nfds, fd_set *readfds, fd_set *writefd
     ctx->interested_except = interested_except;
 
     esp_err_t err = entry->ops->select->start_select(forward_nfds, readfds, writefds, exceptfds, select_sem,
-                                                     &ctx->uart_args);
+                                                     &ctx->sink_select_args);
     if (err != ESP_OK) {
         if (readfds) {
             FD_ZERO(readfds);
@@ -477,7 +557,7 @@ esp_err_t console_end_select(void *end_select_args)
 
     esp_err_t ret = ESP_OK;
     if (entry->fd >= 0 && entry->ops->select && entry->ops->select->end_select) {
-        ret = entry->ops->select->end_select(ctx->uart_args);
+        ret = entry->ops->select->end_select(ctx->sink_select_args);
     }
 
     const int hw = entry->fd;
@@ -599,6 +679,7 @@ static const esp_vfs_fs_ops_t s_vfs_console = {
 esp_err_t esp_stdio_register(void)
 {
     esp_err_t err = ESP_OK;
+    _lock_init(&s_ctx_lock);
 // Primary vfs part.
 #if CONFIG_ESP_CONSOLE_UART
     const esp_vfs_fs_ops_t *ops = esp_vfs_uart_get_vfs();

components/esp_stdio/test_apps/stdio/main/test_app_main.c

@@ -52,7 +52,15 @@ static void console_none_print(void)
 #endif
 
 #if CONFIG_VFS_SUPPORT_IO
-#if CONFIG_ESP_STDIO_MAX_FDS <= 0 || CONFIG_ESP_STDIO_MAX_FDS > 3
+#if CONFIG_ESP_STDIO_BASIC_MODE
+#define ESP_STDIO_RUN_OPEN_CLOSE_CHECK 1
+#elif CONFIG_ESP_STDIO_MAX_FDS > 3
+#define ESP_STDIO_RUN_OPEN_CLOSE_CHECK 1
+#else
+#define ESP_STDIO_RUN_OPEN_CLOSE_CHECK 0
+#endif
+
+#if ESP_STDIO_RUN_OPEN_CLOSE_CHECK
 static void console_open_close_check(void)
 {
     printf("Opening /dev/console\n");
@@ -72,7 +80,7 @@ static void console_open_close_check(void)
 
 static void stdio_fd_mode_behavior_check(void)
 {
-#if CONFIG_ESP_STDIO_MAX_FDS <= 0
+#if CONFIG_ESP_STDIO_BASIC_MODE
     printf("STDIO_TEST:MODE=BASIC\n");
 
     int fd0 = open("/dev/console", O_RDWR);
@@ -87,6 +95,11 @@ static void stdio_fd_mode_behavior_check(void)
     ssize_t wr = write(fd0, msg, strlen(msg));
     assert(wr == (ssize_t) strlen(msg));
 
+    /* Verify write return value matches actual bytes (primary sink propagation) */
+    const char *short_msg = "ab";
+    wr = write(fd1, short_msg, 2);
+    assert(wr == 2);
+
     assert(close(fd0) == 0);
     assert(close(fd1) == 0);
     assert(close(fd2) == 0);
@@ -128,6 +141,35 @@ static void stdio_fd_mode_behavior_check(void)
     assert(close(fd2) == 0);
     assert(close(fd_reused) == 0);
 
+    int fd_ro = open("/dev/console", O_RDONLY);
+    assert(fd_ro >= 0);
+    errno = 0;
+    assert(write(fd_ro, "x", 1) < 0);
+    assert(errno == EBADF);
+    assert(close(fd_ro) == 0);
+
+    int fd_wo = open("/dev/console", O_WRONLY);
+    assert(fd_wo >= 0);
+    errno = 0;
+    char tmp = 0;
+    assert(read(fd_wo, &tmp, 1) < 0);
+    assert(errno == EBADF);
+    assert(close(fd_wo) == 0);
+
+    int fd_rw = open("/dev/console", O_RDWR);
+    assert(fd_rw >= 0);
+    const char *wmsg = "z";
+    ssize_t wret = write(fd_rw, wmsg, 1);
+    assert(wret == 1);
+    assert(close(fd_rw) == 0);
+    printf("STDIO_TEST:NON_BASIC:FLAGS_OK\n");
+
+    int fd_fsync = open("/dev/console", O_RDWR);
+    assert(fd_fsync >= 0);
+    assert(fsync(fd_fsync) == 0);
+    assert(close(fd_fsync) == 0);
+    printf("STDIO_TEST:NON_BASIC:FSYNC_OK\n");
+
     int fds[CONFIG_ESP_STDIO_MAX_FDS + 2];
     int opened = 0;
     while (opened < (int)(sizeof(fds) / sizeof(fds[0]))) {
@@ -230,7 +272,7 @@ void app_main(void)
 #endif // CONFIG_ESP_CONSOLE_NONE
 
 #if CONFIG_VFS_SUPPORT_IO
-#if CONFIG_ESP_STDIO_MAX_FDS <= 0 || CONFIG_ESP_STDIO_MAX_FDS > 3
+#if ESP_STDIO_RUN_OPEN_CLOSE_CHECK
     console_open_close_check();
 #else
     printf("STDIO_TEST:SKIP:OPEN_CLOSE_CHECK\n");

components/esp_stdio/test_apps/stdio/pytest_esp_stdio_tests.py

@@ -100,6 +100,8 @@ def test_esp_system_stdio_correct_open_and_close(dut: Dut) -> None:
     dut.expect('STDIO_TEST:MODE=NON_BASIC')
     dut.expect('STDIO_TEST:NON_BASIC:UNIQUE_OK')
     dut.expect('STDIO_TEST:NON_BASIC:REUSE_OK')
+    dut.expect('STDIO_TEST:NON_BASIC:FLAGS_OK')
+    dut.expect('STDIO_TEST:NON_BASIC:FSYNC_OK')
     dut.expect('STDIO_TEST:NON_BASIC:LIMIT_OK')
     dut.expect('STDIO_TEST:NON_BASIC:EBADF_OK')
 
@@ -118,6 +120,8 @@ def test_esp_stdio_non_basic_default_qemu(dut: Dut) -> None:
     dut.expect('STDIO_TEST:MODE=NON_BASIC')
     dut.expect('STDIO_TEST:NON_BASIC:UNIQUE_OK')
     dut.expect('STDIO_TEST:NON_BASIC:REUSE_OK')
+    dut.expect('STDIO_TEST:NON_BASIC:FLAGS_OK')
+    dut.expect('STDIO_TEST:NON_BASIC:FSYNC_OK')
     dut.expect('STDIO_TEST:NON_BASIC:LIMIT_OK')
     dut.expect('STDIO_TEST:NON_BASIC:EBADF_OK')
 

components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_basic_mode

@@ -1 +1 @@
-CONFIG_ESP_STDIO_MAX_FDS=0
+CONFIG_ESP_STDIO_BASIC_MODE=y

components/esp_stdio/test_apps/stdio/sdkconfig.ci.stdio_non_basic_small_fd

@@ -1 +1,2 @@
+CONFIG_ESP_STDIO_BASIC_MODE=n
 CONFIG_ESP_STDIO_MAX_FDS=3