Skip to content

Conversation

@cederom
Copy link
Contributor

@cederom cederom commented May 27, 2025

Summary

  • Recurrency is removed from filesystem directory rename.
  • Fixes use after free in buffer that was used as output and argument.
  • Patch provided by @richyliu.

Impact

fs/vfs bug fix of possible use after free.

Testing

@cederom: esp32c6-xiao:usbnsh.
@richyliu: STM32 Nucleo H743ZI + FTP.

Reviewers please double check locally if the issue is reproducible and there are no side effects after patch :-)

@github-actions github-actions bot added Area: File System File System issues Size: S The size of the change in this PR is small labels May 27, 2025
@cederom cederom marked this pull request as draft May 27, 2025 20:06
* Recurrency is removed from filesystem directory rename.
* Fixes use after free in buffer that was used as output and argument.

Signed-off-by: Tomasz 'CeDeROM' CEDRO <[email protected]>
@cederom cederom force-pushed the cederom-20250524-vfs_fs_rename_fix branch from 3e74ef1 to af85063 Compare May 27, 2025 20:10
@richyliu
Copy link
Contributor

The following is a demonstration of the use-after-free, combined with the incorrect pseudorename logic, affecting the mv command in NuttX's NuttShell (NSH).

NuttShell (NSH) NuttX-12.8.0
nsh> mkdir c
nsh> mkdir c/a
nsh> mkdir c/a/b
nsh> mkdir c/b
nsh> ls
/:
c/
dev/
nsh> mv c/b c/a
nsh> ls
/:
b/
c/
dev/
nsh>

Due to the previously incorrect logic, when moving the directory c/b to c/a, it first checks for the existence of c/a/b. Seeing that it exists, it then tries to check for c/a/b/b. However, due to the use-after-free, it actually checks for /b, where "" is garbage data from the freed chunk. Due to the internal workings of the snprintf function used, the resultant string becomes "/b", which becomes the new move target of pseudorename. This is why c/b gets moved to /b.

Let me know if there's anything else needed on my end to get this merged.

@cederom cederom marked this pull request as ready for review July 28, 2025 21:48
@cederom cederom self-assigned this Jul 28, 2025
@xiaoxiang781216 xiaoxiang781216 merged commit d2282ec into apache:master Jul 30, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: File System File System issues Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants