jlaitine commented on code in PR #2954:
URL: https://github.com/apache/nuttx-apps/pull/2954#discussion_r2555240249
##########
nshlib/nsh_fscmds.c:
##########
@@ -2234,8 +2233,7 @@ int cmd_rm(FAR struct nsh_vtbl_s *vtbl, int argc, FAR
char **argv)
{
if (recursive)
{
- strlcpy(buf, fullpath, PATH_MAX);
Review Comment:
Hi @xiaoxiang781216, I believe this change was wrong;
In the fullpath is commonly allocated with strdup, e.g. in here:
https://github.com/apache/nuttx-apps/blob/6028b42d0f75aafb97329543636558bf26fc1740/nshlib/nsh_envcmds.c#L206
And in unlink_recursive the file name is added to the fullpath in here:
https://github.com/apache/nuttx-apps/blob/6028b42d0f75aafb97329543636558bf26fc1740/nshlib/nsh_fscmds.c#L2170
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
