You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/08/26 08:24:07 UTC
[GitHub] [incubator-nuttx] michi-jung opened a new pull request, #6928: tmpfs: Avoid potential kmm_free(NULL)
michi-jung opened a new pull request, #6928:
URL: https://github.com/apache/incubator-nuttx/pull/6928
## Summary
If a file on tmpfs is opened, unlinked and closed without writing
anything to the file, then no heap buffer has ever been allocated to
hold the file's content. This leads to a kmm_free(NULL), which
triggers a DEBUGASSERT. This fix avoids this.
## Impact
None
## Testing
Tested on Nucleo-144 STM32U585 board.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6928: tmpfs: Avoid potential kmm_free(NULL)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6928:
URL: https://github.com/apache/incubator-nuttx/pull/6928#discussion_r955800443
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1442,7 +1442,11 @@ static int tmpfs_close(FAR struct file *filep)
* have any other references.
*/
- kmm_free(tfo->tfo_data);
Review Comment:
but kmm_free/mm_free will check NULL for us.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6928: Avoid assertion violation in case of kmm_free(NULL)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6928:
URL: https://github.com/apache/incubator-nuttx/pull/6928#discussion_r955883412
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1442,7 +1442,11 @@ static int tmpfs_close(FAR struct file *filep)
* have any other references.
*/
- kmm_free(tfo->tfo_data);
Review Comment:
Yes, I think so. or it's better to move the assert to mm_free.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6928: tmpfs: Avoid potential kmm_free(NULL)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6928:
URL: https://github.com/apache/incubator-nuttx/pull/6928#discussion_r955800443
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1442,7 +1442,11 @@ static int tmpfs_close(FAR struct file *filep)
* have any other references.
*/
- kmm_free(tfo->tfo_data);
Review Comment:
but kmm_free/mm_free will check NULL for us without assert:
https://github.com/apache/incubator-nuttx/blob/master/mm/mm_heap/mm_free.c#L82
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] michi-jung commented on a diff in pull request #6928: tmpfs: Avoid potential kmm_free(NULL)
Posted by GitBox <gi...@apache.org>.
michi-jung commented on code in PR #6928:
URL: https://github.com/apache/incubator-nuttx/pull/6928#discussion_r955809453
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1442,7 +1442,11 @@ static int tmpfs_close(FAR struct file *filep)
* have any other references.
*/
- kmm_free(tfo->tfo_data);
Review Comment:
It is this line that is causing the assertion violation: [kmm_free.c:55](https://github.com/apache/incubator-nuttx/blob/77a99acc98737a9e38694fa8270de806b4087e53/mm/kmm_heap/kmm_free.c#L55). Perhaps it should be`DEBUGASSERT((mem == NULL) || kmm_heapmember(mem));`?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6928: Avoid assertion violation in case of kmm_free(NULL)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6928:
URL: https://github.com/apache/incubator-nuttx/pull/6928
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6928: Avoid assertion violation in case of kmm_free(NULL)
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6928:
URL: https://github.com/apache/incubator-nuttx/pull/6928#discussion_r955883412
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1442,7 +1442,11 @@ static int tmpfs_close(FAR struct file *filep)
* have any other references.
*/
- kmm_free(tfo->tfo_data);
Review Comment:
Yes, I think so, it's legal to call free(NULL). or it's better to move the assert to mm_free.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org