fs: Fix null pointer exception caused by async fs_unmount #73507
Conversation
|
Hello @cdesjardins, and thank you very much for your first pull request to the Zephyr project! |
|
This should be configurable by user as it is up to user setup, mostly, where mounted fs is used by more than one thread. Please also take a look at this comment #73507 (comment) regarding commit message format. Thanks. |
subsys/fs/fs.c
Outdated
There was a problem hiding this comment.
This is not correct mutext to use. This one is used for protecting mount points so that when path to FS is figured out the mount list do not get removed/added.
Using this mutex for ops like fs_open means that you are globally locking VFS to perform operation of some mount point, so if you take a lot of time to write to one mount point the other thread has to wait on own fs_ op, even if that is applied to different mount point.
cdesjardins
force-pushed
the
main
branch
2 times, most recently
from
f5c0c72 to
1d38e82
Compare
The mp pointer is in fs_file_t and fs_dir_t so if the fs pointer is made NULL then subsequent file I/O operations will cause a NULL pointer exception. Removing the mount point from the list is threadsafe and should be sufficient. Signed-off-by: Chris Desjardins <[email protected]>
|
Thanks for your comments, and indeed I believe there is a much simpler solution which I have tested on my platform and pushed. |
The issue you have logged is still correct. |
| goto unmount_err; | ||
| } | ||
|
|
||
| /* clear file system interface */ |
There was a problem hiding this comment.
I understand that this fixes problem with NULL pointer exception but unfortunately now systems continue write to system that is no longer valid.
There was a problem hiding this comment.
In my testing the i/o errors out and returns an error code, which is a huge improvement over the NULL pointer exception.
There was a problem hiding this comment.
Remove the "Fixes #73501" from commit and description as this does not fix the problem, it only makes it manifest in a less violent way.
There was a problem hiding this comment.
But it does fix "the problem", which is NULL pointer dereference caused by a user removing a usb drive during file i/o.
There was a problem hiding this comment.
@de-nordic Please check #78018 as an alternative.
|
@de-nordic please revisit |
de-nordic
left a comment
de-nordic
left a comment
There was a problem hiding this comment.
OK, let it be.
It still does not make the FS thread safe, but reduces fallout from unmounting system somebody else is using.
A proper way would be to not allow unmounting system being used by somebody else
I think that #78018 may deliver that, once completed.
pdgendt
changed the title
7 participants
If fs_unmount is invoked while fs i/o is ongoing then it will cause a NULL pointer exception due to the unchecked and unprotected dereference of the fs pointer.
Fixes #73501