Skip to content

Conversation

@jlaitine
Copy link
Contributor

Summary

This fixes heap corruption when deleting a folder containing other folders or files. The issue appeared at commit 131d50a, which removed the stack-based temporary buffer.

unlink_recursive requires that the path is provided in PATH_MAX sized buffer. It concatenates sub-folder or file names to the same buffer.

nsh_getfullpath just allocates a buffer using strdup, so there is no room for concatenating more data to it.

To keep the stack usage smaller, instead of reverting the breaking commit, allocate the temporary buffer with lib_get_pathbuffer instead.

Impact

Fixes "rm -rf " in nsh.

Testing

Tested on imx9 hardware in CONFIG_BUILD_FLAT after mounting sdcard on /fs/microsd and executing the following sequence:

echo "Testing folder delete"
mkdir /fs/microsd/reliability_test_dir
echo 1 > /fs/microsd/reliability_test_dir/odd.txt
rm  -rf /fs/microsd/reliability_test_dir

With this patch, the sequence above can be executed many times.

Without the fix, the system crashes:

Testing folder delete
[CPU0] dump_assert_info: Current Version: NuttX  12.11.0 31e8758e1e Nov 25 2025 13:38:15 arm64
[CPU0] dump_assert_info: Assertion failed panic: at file: common/arm64_fatal.c:572 task(CPU0): nsh_main process: nsh_main 0x80218418
[CPU0] up_dump_register: stack = 0x8058a200
[CPU0] up_dump_register: x0:   0x30                x1:   0xc0
[CPU0] up_dump_register: x2:   0x80630d40          x3:   0x100
[CPU0] up_dump_register: x4:   0x80630d70          x5:   0x80630e70
[CPU0] up_dump_register: x6:   0x80630dc0          x7:   0x0
[CPU0] up_dump_register: x8:   0x80566540          x9:   0x80543bf0
[CPU0] up_dump_register: x10:  0x8                 x11:  0x4f8
[CPU0] up_dump_register: x12:  0x0                 x13:  0x0
[CPU0] up_dump_register: x14:  0x0                 x15:  0x0
[CPU0] up_dump_register: x16:  0x7ffffffe          x17:  0x0
[CPU0] up_dump_register: x18:  0x0                 x19:  0x8057f000
[CPU0] up_dump_register: x20:  0x80630d50          x21:  0x0
[CPU0] up_dump_register: x22:  0x80630d50          x23:  0x1
[CPU0] up_dump_register: x24:  0x1                 x25:  0x18
[CPU0] up_dump_register: x26:  0x8058a780          x27:  0x80495000
[CPU0] up_dump_register: x28:  0x80600944          x29:  0x0
[CPU0] up_dump_register: x30:  0x8020706c        
[CPU0] up_dump_register: 
[CPU0] up_dump_register: STATUS Registers:
[CPU0] up_dump_register: SPSR:      0x60000005        
[CPU0] up_dump_register: ELR:       0x802070c4        
[CPU0] up_dump_register: SP_EL0:    0x8058b200        
[CPU0] up_dump_register: SP_ELX:    0x8058a540        
[CPU0] up_dump_register: EXE_DEPTH: 0xfffffffffffffc58
[CPU0] up_dump_register: SCTLR_EL1: 0x30d0180d        
[CPU0] dump_fatal_info: Dump CPU1: PAUSED
[CPU0] up_dump_register: stack = 0x80542620
[CPU0] up_dump_register: x0:   0x240               x1:   0x80542000
[CPU0] up_dump_register: x2:   0x80540d68          x3:   0x80540d68
[CPU0] up_dump_register: x4:   0x80540000          x5:   0x80540fd8
[CPU0] up_dump_register: x6:   0x0                 x7:   0x1
[CPU0] up_dump_register: x8:   0x820a              x9:   0x80577708
[CPU0] up_dump_register: x10:  0x0                 x11:  0x0
[CPU0] up_dump_register: x12:  0x0                 x13:  0x16
[CPU0] up_dump_register: x14:  0x0                 x15:  0x0
[CPU0] up_dump_register: x16:  0x0                 x17:  0x0
[CPU0] up_dump_register: x18:  0x0                 x19:  0x0
[CPU0] up_dump_register: x20:  0x0                 x21:  0x0
[CPU0] up_dump_register: x22:  0x0                 x23:  0x80200090
[CPU0] up_dump_register: x24:  0x8057ec80          x25:  0x80202268
[CPU0] up_dump_register: x26:  0x0                 x27:  0x0
[CPU0] up_dump_register: x28:  0x0                 x29:  0x0
[CPU0] up_dump_register: x30:  0x80210684        
[CPU0] up_dump_register: 
[CPU0] up_dump_register: STATUS Registers:
[CPU0] up_dump_register: SPSR:      0x60000245        
[CPU0] up_dump_register: ELR:       0x80223e28        
[CPU0] up_dump_register: SP_EL0:    0x8057ec80        
[CPU0] up_dump_register: SP_ELX:    0x8057ec70        
[CPU0] up_dump_register: EXE_DEPTH: 0xfffffffffffffa91
[CPU0] up_dump_register: SCTLR_EL1: 0x30d0180d        

…rsive

This fixes heap corruption when deleting a folder containing other folders
or files. The issue appeared at commit 131d50a, which removed the
stack-based temporary buffer.

unlink_recursive requires that the path is provided in PATH_MAX sized
buffer. It concatenates sub-folder or file names to the same buffer.

nsh_getfullpath just allocates a buffer using strdup, so there is no room
for concatenating more data to it.

To keep the stack usage smaller, instead of reverting the breaking commit,
allocate the temporary buffer with lib_get_pathbuffer instead.

Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jlaitine =)

@linguini1 linguini1 merged commit c3247ec into apache:master Nov 26, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants