Reduce nsh stack consumption

[xiaoxiang781216]

Summary

• nshlib: Remove the unnecessary temp path buffer
• nshlib: Replace the big temp buffer with lib_get_[path|temp]buffer

Impact

• stack consumption decrease.
• nshlib: nsh_fscmds, nsh_fsutils, nsh_timcmds.

Testing

sim:nsh

[nuttxpr]

[Experimental Bot, please feedback here]

No, this PR summary does not fully meet the NuttX requirements. While it addresses what is changed, it lacks crucial details about why and how. Here's a breakdown:

Missing from Summary:

• Why: The summary states what was removed/replaced (temp path buffer, big temp buffer), but not why this was necessary. Was it causing a bug? Improving performance? Reducing memory usage? This is critical for reviewers to understand the motivation behind the change.
• How: While it mentions the replacement functions (lib_get_[path|temp]buffer), it doesn't explain how these functions address the issue. Do they allocate memory differently? Are they more efficient? What's the difference between the old and new approach?

Missing from Impact:

• Specificity: "minor" is not sufficient. While the impact might be minor, you still need to explicitly answer each question with YES/NO and provide descriptions where necessary. For example:
  • Impact on user: NO
  • Impact on build: NO
  • Impact on hardware: NO
  • Impact on documentation: Possibly YES if the API changes require documentation updates.
  • Impact on security: Possibly YES if the previous buffer handling had security implications (e.g., buffer overflows). Explain why or why not.
  • Impact on compatibility: NO (presumably, but state this explicitly)

Missing from Testing:

• Insufficient Detail: sim:nsh is not enough information. Specify:
  • Build Host: e.g., Linux (Ubuntu 22.04), x86_64, GCC 11.2.0
  • Target: e.g., sim:nsh is okay, but more context might be helpful. Which simulator? QEMU?
• Missing Logs: The "Testing logs before change" and "Testing logs after change" sections are empty. Include actual logs demonstrating the issue before the change and the improved behavior after the change. Even if the change is small, show something. For example, commands you ran in NSH and their output.

Example of an improved summary:

## Summary

This PR addresses potential buffer overflow issues and improves memory management in nshlib by removing a large, statically allocated temporary path buffer. This buffer was previously used for various path manipulations and presented a risk of overflow if unexpectedly long paths were encountered. The change replaces the static buffer with calls to `lib_get_pathbuffer()` and `lib_get_tempbuffer()`. These functions dynamically allocate buffer space as needed, eliminating the fixed size limitation and the associated overflow risk.  This also improves memory efficiency by only allocating the required space.

Relates to [NuttX Issue #XXX](If applicable)

## Impact

* Is new feature added? NO
* Is existing feature changed? YES (Improved memory management in nshlib path handling)
* Impact on user: NO
* Impact on build: NO
* Impact on hardware: NO
* Impact on documentation: NO (If no API changes) / YES (If API changes, explain what needs updating)
* Impact on security: YES (The previous static buffer posed a potential buffer overflow risk. This change eliminates that risk).
* Impact on compatibility: NO
* Anything else to consider?  This change improves the robustness and security of path handling within nshlib.


## Testing

Build Host: Linux (Ubuntu 22.04), x86_64, GCC 11.2.0
Target: QEMU sim:nsh

Testing logs before change:

nsh> pwd // Demonstrating some path-related command and its output before the change
/
nsh> mkdir -p /very/long/path/that/might/cause/issues
... (Output showing the error or unexpected behavior, if any)

Testing logs after change:

nsh> pwd // The same command after the change
/
nsh> mkdir -p /very/long/path/that/might/cause/issues
... (Output showing the correct behavior after the change)

[cederom]

Thank you @xiaoxiang781216 :-)

[GUIDINGLI]

why not get at line 595 & put at line 615 ?

[xiaoxiang781216]

because line 613 call sleep, which may sleep for a long time.

[GUIDINGLI]

LGTM

[jlaitine]

Hi @xiaoxiang781216, I believe this change was wrong;

In the fullpath is commonly allocated with strdup, e.g. in here:

nuttx-apps/nshlib/nsh_envcmds.c

Line 206 in 6028b42

return strdup(relpath);

And in unlink_recursive the file name is added to the fullpath in here:

nuttx-apps/nshlib/nsh_fscmds.c

Line 2170 in 6028b42

snprintf(&path[len], PATH_MAX - len, "/%s", d->d_name);

But, the size of the allocated buffer is actually just "len + 1" on the above line, since the buffer originates from strdup.

So the temporary buffer in here is not useless, it is absolutely needed.

I just found this out when I tried "rm -rf /fs/sdcard/folder", with file in in "folder". This corrupts heap and results in full system crash.

[xiaoxiang781216]

yes, you are right, we need call lib_get_pathbuffer.

[cederom]

Ah I noticed that recently with simpler rm -rf / :-P Should we revert?

[jlaitine]

Perhaps do what @xiaoxiang781216 proposed, instead of allocating the buffer from the stack, we could use lib_get_pathbuffer to keep stack usage smaller. I can make a patch for this if so wanted...

[jlaitine]

I created #3222 for the issue. @xiaoxiang781216 , @cederom please have a look

[xiaoxiang781216]

@jlaitine thanks to fix my fault.

[cederom]

Thank you @jlaitine !! No worries @xiaoxiang781216 !! :-) <3