[HUST CSE][dfs] fix stack buffer overflow on long path components in devtmpfs

components/dfs/dfs_v2/filesystems/devfs/devtmpfs.c

@@ -54,13 +54,23 @@ struct devtmpfs_sb
 
 static struct dfs_file_ops _default_fops = { 0 };
 
-static int _path_separate(const char *path, char *parent_path, char *file_name)
+static int _path_separate(const char *path, char *parent_path, rt_size_t parent_size,
+                          char *file_name, rt_size_t file_size)
 {
     const char *path_p, *path_q;
+    rt_size_t parent_len, file_len;
+
+    /* validate the arguments before the path is dereferenced below */
+    if (path == RT_NULL || parent_path == RT_NULL || file_name == RT_NULL ||
+        parent_size < 2 || file_size == 0)
+    {
+        return -EINVAL;
+    }
 
     RT_ASSERT(path[0] == '/');
 
     file_name[0] = '\0';
+    parent_path[0] = '\0';
     path_p = path_q = &path[1];
 __next_dir:
     while (*path_q != '/' && *path_q != '\0')
@@ -77,10 +87,17 @@ static int _path_separate(const char *path, char *parent_path, char *file_name)
         }
         else /* Last level dir */
         {
-            rt_memcpy(parent_path, path, path_p - path - 1);
-            parent_path[path_p - path - 1] = '\0';
-            rt_memcpy(file_name, path_p, path_q - path_p);
-            file_name[path_q - path_p] = '\0';
+            parent_len = path_p - path - 1;
+            file_len = path_q - path_p;
+            if (parent_len >= parent_size || file_len >= file_size)
+            {
+                return -ENAMETOOLONG;
+            }
+
+            rt_memcpy(parent_path, path, parent_len);
+            parent_path[parent_len] = '\0';
+            rt_memcpy(file_name, path_p, file_len);
+            file_name[file_len] = '\0';
         }
     }
     if (parent_path[0] == 0)
@@ -94,17 +111,31 @@ static int _path_separate(const char *path, char *parent_path, char *file_name)
     return 0;
 }
 
-static int _get_subdir(const char *path, char *name)
+static int _get_subdir(const char *path, char *name, rt_size_t name_size)
 {
     const char *subpath = path;
+    rt_size_t name_len = 0;
+
+    if (path == RT_NULL || name == RT_NULL || name_size == 0)
+    {
+        return -EINVAL;
+    }
+
     while (*subpath == '/' && *subpath)
         subpath ++;
     while (*subpath != '/' && *subpath)
     {
+        if (name_len + 1 >= name_size)
+        {
+            name[0] = '\0';
+            return -ENAMETOOLONG;
+        }
         *name = *subpath;
         name ++;
         subpath ++;
+        name_len++;
     }
+    *name = '\0';
     return 0;
 }
 
@@ -213,7 +244,10 @@ static struct devtmpfs_file *devtmpfs_file_lookup(struct devtmpfs_sb *superblock
         subpath ++; /* skip '/' */
 
     memset(subdir_name, 0, DIRENT_NAME_MAX);
-    _get_subdir(curpath, subdir_name);
+    if (_get_subdir(curpath, subdir_name, sizeof(subdir_name)) != 0)
+    {
+        return NULL;
+    }
 
     rt_spin_lock(&superblock->lock);
 
@@ -490,6 +524,7 @@ static struct dfs_vnode *devtmpfs_create_vnode(struct dfs_dentry *dentry, int ty
     struct devtmpfs_sb *superblock;
     struct devtmpfs_file *d_file, *p_file;
     char parent_path[DFS_PATH_MAX], file_name[DIRENT_NAME_MAX];
+    int ret;
 
     if (dentry == NULL || dentry->mnt == NULL || dentry->mnt->data == NULL)
     {
@@ -503,7 +538,13 @@ static struct dfs_vnode *devtmpfs_create_vnode(struct dfs_dentry *dentry, int ty
     if (vnode)
     {
         /* find parent file */
-        _path_separate(dentry->pathname, parent_path, file_name);
+        ret = _path_separate(dentry->pathname, parent_path, sizeof(parent_path),
+                             file_name, sizeof(file_name));
+        if (ret != RT_EOK)
+        {
+            dfs_vnode_destroy(vnode);
+            return NULL;
+        }
         if (file_name[0] == '\0') /* it's root dir */
         {
             dfs_vnode_destroy(vnode);

components/dfs/utest/Kconfig

@@ -41,6 +41,17 @@ if RT_USING_DFS
 
         endif
 
+    config RT_UTEST_TC_USING_DEVTMPFS_PATH
+        bool "devtmpfs path component boundary Test"
+        default n
+        depends on RT_USING_DFS_V2
+        help
+            Enable boundary regression tests for devtmpfs path components.
+            Verifies that a path component of DIRENT_NAME_MAX bytes or more is
+            rejected with -ENAMETOOLONG instead of overrunning the stack buffers
+            used by devtmpfs_create_vnode() and devtmpfs_file_lookup(), and that
+            shorter components keep working. Requires devtmpfs mounted on /dev.
+
     rsource "v2/af_unix/Kconfig"
 
     endmenu

components/dfs/utest/SConscript

@@ -15,6 +15,10 @@ if GetDepend('RT_UTEST_TC_USING_DFS_API'):
     if GetDepend('RT_UTEST_TC_USING_POSIX_API'):
         src += ['tc_posix_api.c']
 
+# Add devtmpfs path component boundary test if enabled (requires DFS v2)
+if GetDepend('RT_UTEST_TC_USING_DEVTMPFS_PATH'):
+    src += ['tc_devtmpfs_path.c']
+
 # Define the test group with proper dependencies
 objs = DefineGroup('utestcases', src, depend = ['RT_USING_UTESTCASES', 'RT_USING_DFS'], CPPPATH = CPPPATH)
 objs = objs + SConscript(os.path.join('v2', 'af_unix', 'SConscript'))

components/dfs/utest/tc_devtmpfs_path.c

@@ -0,0 +1,246 @@
+/*
+ * Copyright (c) 2006-2025, RT-Thread Development Team
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Change Logs:
+ * Date           Author       Notes
+ * 2026-10-17     Yaodong Han  boundary regression tests for devtmpfs path
+ *                             components (devtmpfs_create_vnode/_path_separate
+ *                             and devtmpfs_file_lookup/_get_subdir)
+ */
+
+#include <rtthread.h>
+#include "utest.h"
+
+#include <dfs_file.h>
+#include <dfs.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#ifdef RT_USING_DFS_V2
+
+/*
+ * The devtmpfs helpers copy the last path component into char[DIRENT_NAME_MAX]
+ * buffers, so a component of DIRENT_NAME_MAX bytes or more must be rejected
+ * instead of overrunning the caller's stack frame. A component of
+ * DIRENT_NAME_MAX - 1 bytes (plus the terminator) still fits.
+ */
+#define TC_NAME_MAX DIRENT_NAME_MAX
+
+/* devtmpfs is mounted on /dev by the device filesystem, see devtmpfs_init() */
+#define TC_MNT_PATH "/dev"
+
+/* Path buffer: mount point + '/' + component + '\0'.
+ * Sized for the longest component the boundary sweep below uses (300 bytes),
+ * which is deliberately well past DIRENT_NAME_MAX. */
+#define TC_MAX_TEST_LEN  300
+#define TC_PATH_BUF_SIZE (sizeof(TC_MNT_PATH) + 1 + TC_MAX_TEST_LEN + 1)
+
+static char tc_path[TC_PATH_BUF_SIZE];
+
+/*
+ * Caller-side stack guard. devtmpfs_create_vnode() and devtmpfs_file_lookup()
+ * keep their file_name[]/subdir_name[] buffers on their own frames; if the
+ * component were copied without a bounds check those frames - and the data
+ * that survives in the caller after the call - would be corrupted. Keeping a
+ * known pattern around the call and checking it afterwards is the observable
+ * "does not modify neighbouring stack data" assertion requested in review.
+ */
+#define TC_GUARD_SIZE    32
+#define TC_GUARD_PATTERN 0xA5
+#define TC_GUARD_MAGIC   0x5A5A1234UL
+
+static volatile rt_ubase_t tc_guard_magic;
+
+static void tc_guard_fill(rt_uint8_t *guard)
+{
+    rt_memset(guard, TC_GUARD_PATTERN, TC_GUARD_SIZE);
+    tc_guard_magic = TC_GUARD_MAGIC;
+}
+
+static rt_bool_t tc_guard_intact(const rt_uint8_t *guard)
+{
+    rt_size_t i;
+
+    if (tc_guard_magic != TC_GUARD_MAGIC)
+    {
+        return RT_FALSE;
+    }
+
+    for (i = 0; i < TC_GUARD_SIZE; i++)
+    {
+        if (guard[i] != TC_GUARD_PATTERN)
+        {
+            return RT_FALSE;
+        }
+    }
+
+    return RT_TRUE;
+}
+
+/*
+ * Build "<mnt>/<len x 'A'>" into tc_path and return its length.
+ * The mount point is taken from RT_UTEST_DFS_MNT_PATH so the test works for
+ * whatever filesystem/mount point the utest configuration uses.
+ */
+static rt_size_t tc_build_path(rt_size_t len)
+{
+    rt_size_t mnt_len = rt_strlen(TC_MNT_PATH);
+    rt_size_t i;
+
+    RT_ASSERT(len <= TC_MAX_TEST_LEN);
+
+    rt_memcpy(tc_path, TC_MNT_PATH, mnt_len);
+    tc_path[mnt_len] = '/';
+    for (i = 0; i < len; i++)
+    {
+        tc_path[mnt_len + 1 + i] = 'A';
+    }
+    tc_path[mnt_len + 1 + len] = '\0';
+
+    return mnt_len + 1 + len;
+}
+
+/*
+ * Exercise both entry points for one component length:
+ *   - devtmpfs_file_lookup()  through dfs_file_stat()
+ *   - devtmpfs_create_vnode() through open(O_CREAT)
+ * and verify that neither one touches neighbouring stack data.
+ *
+ * Returns the open() result (-1 on failure).
+ */
+static int tc_roundtrip(rt_size_t len, rt_bool_t expect_ok)
+{
+    rt_uint8_t guard[TC_GUARD_SIZE];
+    struct stat st;
+    int fd;
+
+    tc_build_path(len);
+    tc_guard_fill(guard);
+
+    /* lookup path first: the node does not exist yet, this must not crash */
+    (void)dfs_file_stat(tc_path, &st);
+
+    /* creation path: this is where _path_separate() runs */
+    fd = open(tc_path, O_CREAT | O_RDWR, 0644);
+
+    if (!tc_guard_intact(guard))
+    {
+        rt_kprintf("guard corrupted for component length %u\n",
+                   (unsigned int)len);
+        return -2;
+    }
+
+    if (expect_ok)
+    {
+        if (fd < 0)
+        {
+            rt_kprintf("expected success for component length %u, got %d\n",
+                       (unsigned int)len, fd);
+            return -3;
+        }
+        close(fd);
+
+        /* the node must now be visible through the lookup path */
+        tc_guard_fill(guard);
+        if (dfs_file_stat(tc_path, &st) != 0)
+        {
+            if (tc_guard_intact(guard))
+            {
+                rt_kprintf("node not found after creation, len %u\n",
+                           (unsigned int)len);
+                return -4;
+            }
+            return -2;
+        }
+    }
+    else
+    {
+        if (fd >= 0)
+        {
+            close(fd);
+            rt_kprintf("expected rejection for component length %u\n",
+                       (unsigned int)len);
+            return -5;
+        }
+
+        /* a rejected component must not have created anything either */
+        tc_guard_fill(guard);
+        if (dfs_file_stat(tc_path, &st) == 0)
+        {
+            if (tc_guard_intact(guard))
+            {
+                rt_kprintf("overlong component still resolved, len %u\n",
+                           (unsigned int)len);
+                return -6;
+            }
+            return -2;
+        }
+    }
+
+    return tc_guard_intact(guard) ? 0 : -2;
+}
+
+/* len == DIRENT_NAME_MAX - 2: comfortably inside the buffer */
+static void test_devtmpfs_component_254(void)
+{
+    uassert_int_equal(tc_roundtrip(TC_NAME_MAX - 2, RT_TRUE), 0);
+}
+
+/* len == DIRENT_NAME_MAX - 1: the last length that still fits (+ terminator) */
+static void test_devtmpfs_component_255(void)
+{
+    uassert_int_equal(tc_roundtrip(TC_NAME_MAX - 1, RT_TRUE), 0);
+}
+
+/* len == DIRENT_NAME_MAX: no room for the terminator, must be rejected */
+static void test_devtmpfs_component_256(void)
+{
+    uassert_int_equal(tc_roundtrip(TC_NAME_MAX, RT_FALSE), 0);
+}
+
+/* well past the buffer: must be rejected as well */
+static void test_devtmpfs_component_300(void)
+{
+    uassert_int_equal(tc_roundtrip(TC_NAME_MAX + 44, RT_FALSE), 0);
+}
+
+/* a normal, short component must keep working (no regression) */
+static void test_devtmpfs_component_short(void)
+{
+    uassert_int_equal(tc_roundtrip(8, RT_TRUE), 0);
+}
+
+static rt_err_t utest_tc_init(void)
+{
+    struct stat st;
+
+    /* devfs is mounted by the system; make sure the mount point exists */
+    if (dfs_file_stat(TC_MNT_PATH, &st) != 0)
+    {
+        rt_kprintf("mount point %s not available, devtmpfs test skipped\n",
+                   TC_MNT_PATH);
+        return -RT_ERROR;
+    }
+
+    return RT_EOK;
+}
+
+static rt_err_t utest_tc_cleanup(void)
+{
+    /* the successful cases only create device nodes; nothing is persisted */
+    return RT_EOK;
+}
+
+static void testcase(void)
+{
+    UTEST_UNIT_RUN(test_devtmpfs_component_short);
+    UTEST_UNIT_RUN(test_devtmpfs_component_254);
+    UTEST_UNIT_RUN(test_devtmpfs_component_255);
+    UTEST_UNIT_RUN(test_devtmpfs_component_256);
+    UTEST_UNIT_RUN(test_devtmpfs_component_300);
+}
+UTEST_TC_EXPORT(testcase, "components.dfs.devtmpfs_path_tc", utest_tc_init, utest_tc_cleanup, 30);
+
+#endif /* RT_USING_DFS_V2 */