diff --git a/components/dfs/dfs_v2/filesystems/devfs/devtmpfs.c b/components/dfs/dfs_v2/filesystems/devfs/devtmpfs.c index b60e5c5d0f1c..5764eb877b9c 100644 --- a/components/dfs/dfs_v2/filesystems/devfs/devtmpfs.c +++ b/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); diff --git a/components/dfs/utest/Kconfig b/components/dfs/utest/Kconfig index 32acadbf1a70..94ad954366ac 100644 --- a/components/dfs/utest/Kconfig +++ b/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 diff --git a/components/dfs/utest/SConscript b/components/dfs/utest/SConscript index bb8fa66a35d4..e6c5bf6caed2 100644 --- a/components/dfs/utest/SConscript +++ b/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')) diff --git a/components/dfs/utest/tc_devtmpfs_path.c b/components/dfs/utest/tc_devtmpfs_path.c new file mode 100644 index 000000000000..1cb215be3c96 --- /dev/null +++ b/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 +#include "utest.h" + +#include +#include +#include +#include + +#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 "/" 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 */