fs: POSIX filesystem bug fixes (rename/canonicalize/LINK_MAX) - #19991
Merged
Conversation
Kaben123
requested review from
Donny9,
GUIDINGLI,
Ouss4,
hartmannathan,
jerpelea,
pkarashchenko and
xiaoxiang781216
as code owners
August 28, 2026 07:12
github-actions
Bot
added
Area: File System
|
Kaben123
force-pushed
the
pse52-04-fixes
branch
2 times, most recently
from
August 28, 2026 07:47
488daee to
f73a936
Compare
Replace fs_heap_malloc/free with lib_get_tempbuffer/lib_put_tempbuffer in smartfs_finddirentry(). This aligns smartfs with the common VFS tempbuffer allocation pattern and is a prerequisite for the canonicalization cleanup that removes redundant ".." handling from individual filesystem layers. Signed-off-by: zhaoxingyu1 <zhaoxingyu1@xiaomi.com>
The loop condition 'namelen <= NAME_MAX' allowed filenames of NAME_MAX+1 characters to pass validation. When the filename segment reached exactly NAME_MAX+1 chars and was at the end of the path string, the loop exited due to *path == '\0' and returned OK instead of -ENAMETOOLONG. Fix by moving the NAME_MAX check inside the loop body with an immediate return on violation. Also fix the post-loop return to explicitly check pathlen >= PATH_MAX instead of relying on *path which conflated the two exit conditions. Before: creat() with 97-char filename (NAME_MAX=96) succeeded After: creat() with 97-char filename correctly returns ENAMETOOLONG Signed-off-by: yukangzhi <yukangzhi@xiaomi.com>
Fix two POSIX compliance issues in mountptrename():
1. When old and new are hard links to the same file (same st_dev and
st_ino), POSIX requires rename() to succeed without removing either
link. Previously, NuttX would unlink(new) then rename(old, new),
effectively losing one link. Fix by comparing inode identity before
any destructive operation.
2. When new is a subdirectory of old (e.g., rename('a', 'a/b')), POSIX
requires EINVAL. Previously, NuttX would rmdir(new) first, then the
filesystem's rename() would fail -- but new was already deleted,
causing data loss. Fix by detecting the subdirectory relationship
(newrelpath starts with oldrelpath + '/') before any rmdir/unlink.
Signed-off-by: yukangzhi <yukangzhi@xiaomi.com>
Add _inode_canonicalize() to remove '.' and '..' segments from the
absolute path before the inode tree traversal begins. This fixes the
case where paths containing '..' that resolve back to a mountpoint
root (e.g., /tmp/subdir/..) were not being handed to the filesystem.
Previously, _compute_path_depth() returned 0 for such paths, causing
the VFS to skip the mountpoint and attempt to find 'subdir' in the
pseudo filesystem -- which fails with ENOTDIR.
With canonicalization, /tmp/subdir/.. becomes /tmp before the search,
so the mountpoint is correctly matched. This fixes chdir('..'),
stat('../..'), opendir('../..'), and similar operations from within
mountpoint subdirectories.
The implementation uses an in-place two-pointer algorithm with no
additional stack allocation, safe for NuttX's small kernel stacks.
Signed-off-by: yukangzhi <yukangzhi@xiaomi.com>
Since _inode_canonicalize() now resolves all "." and ".." segments in the common VFS layer before inode search, the relpath passed to each filesystem will never contain ".." segments. Remove the now-dead ".." handling code from individual filesystem layers and the inode search internals. Files modified (redundant ".." path resolution removed): - fs/hostfs/hostfs.c: remove depth-tracking escape check in hostfs_mkpath(), simplify to direct path concatenation. - fs/rpmsgfs/rpmsgfs.c: same as hostfs, remove depth-tracking in rpmsgfs_mkpath(). - fs/smartfs/smartfs_utils.c: remove "." and ".." segment checks in smartfs_finddirentry(), de-indent the remaining search logic. - fs/inode/fs_inodesearch.c: remove _inode_isdotdot() function, simplify _compute_path_depth() to only count forward segments, remove dead else-if branch in _inode_search(). Files NOT modified (and why): - fs/littlefs/littlefs/lfs.c: third-party upstream library (git submodule), must not be modified locally. - fs/fatfs/fatfs/source/ff.c: third-party upstream library. - fs/lwext4/lwext4/src/ext4*.c: third-party upstream library. - fs/cromfs/fs_cromfs.c: handles "." and ".." as directory entries (structural, not path resolution), so its code stays. - fs/vfs/fs_symlink.c: constructs relative paths containing ".." (writes, not parses relpath). Signed-off-by: yukangzhi <yukangzhi@xiaomi.com>
Root cause: _inode_search() built the absolute form of a relative path with snprintf(buf, PATH_MAX, "%s/%s", cwd, path), silently truncating it when cwd + "/" + path exceeded PATH_MAX. The truncated buffer was then handed to _inode_canonicalize(), which collapsed ".." segments against the wrong cut-off suffix. A valid relative path of PATH_MAX-1 bytes (legal per pathconf(_PC_PATH_MAX)) could thus collapse onto a directory and open() returned EISDIR instead of resolving the file. Fix: size the temp buffer to hold the full uncanonicalized "<cwd>/<path>" form so canonicalization sees the complete path. lib_get_tempbuffer falls back to a malloc'd buffer when the size exceeds PATH_MAX (CONFIG_LIBC_TEMPBUFFER_MALLOC). The existing PATH_MAX check in _inode_canonicalize() still rejects any canonicalized result that is too long, so ENAMETOOLONG semantics are preserved. Signed-off-by: dengwenqi <dengwenqi@xiaomi.com>
When inode_find() for path2 fails due to ENAMETOOLONG (or ELOOP), the else branch incorrectly falls through to the EXDEV check based on whether target is a mountpoint. This causes link() to report EXDEV for overly long path2, violating POSIX which requires ENAMETOOLONG in this case. Fix by propagating the original inode_find() error code when it is not ENOENT or ENOTDIR (i.e., not a simple "path does not exist" condition). Signed-off-by: yukangzhi <yukangzhi@xiaomi.com>
Increase LINK_MAX from _POSIX_LINK_MAX (8) to 128 to allow directories to have a reasonable number of subdirectories while still enforcing a hard link limit. Also fix pathconf(_PC_LINK_MAX) to return the actual LINK_MAX value instead of the minimum _POSIX_LINK_MAX. Signed-off-by: yukangzhi <yukangzhi@xiaomi.com>
Fix coding style violations detected by CI whole-file nxstyle scan: - fs/smartfs/smartfs_utils.c: add missing braces after if (L334), fix bad alignment (L414), fix switch brace alignment (L1531) - fs/hostfs/hostfs.c: add blank line after declaration (L576) These are pre-existing style issues in master, not introduced by this PR, but reported because CI checks the entire touched file. Signed-off-by: yukangzhi <yukangzhi@xiaomi.com>
Kaben123
force-pushed
the
pse52-04-fixes
branch
from
August 28, 2026 07:54
f73a936 to
f223fec
Compare
🔗 Cross-repo PR dependenciesThe read-only Build run reported the following dependent PR(s) and fetched head SHA(s): CI run: https://github.com/apache/nuttx/actions/runs/33153204810 |
Contributor
|
@Kaben123 please verify why this PR is increasing from +124 to +672 in some boards |
Author
Thanks @acassis. The variance comes from per-board Kconfig, not from bloat in the patch:
MemBrowse per-symbol data confirms the growth is confined to the functions this PR touches (e.g. qemu-armv8a: .text.inode_search +256, .text.rename +84) — no unexpected symbols, no duplicated inlining. |
Contributor
Thank you, the real increase is +124, right? |
acassis
approved these changes
Aug 28, 2026
xiaoxiang781216
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR contains 8 bug-fix commits for the NuttX POSIX filesystem layer:
_inode_checkpathNAME_MAX check..resolution..handling in FS layers after VFS canonicalizationlink()returning EXDEV instead of ENAMETOOLONGpathconfDependencies
This PR depends on:
Depends-On: #19990
Testing
sim:nshwithFS_HOSTFS + FS_LINKS + FS_SMARTFS, zero errors/warningsImpact
..resolution across mount boundarieslink()error code confusion (EXDEV vs ENAMETOOLONG)..handling now handled by VFS layer