liamHowatt opened a new pull request, #17821:
URL: https://github.com/apache/nuttx/pull/17821
```
Fix a race on userfs_state_s iobuffer.
1. Task 1 takes the mutex and does a userfs operation.
2. A higher priority task 2 blocks on the mutex.
3. Task 1 releases the mutex when finished. The iobuffer response has not
been processed by task 1 yet, but task 2 is higher priority
and control switches to it.
4. Task 2 does a userfs operation and releases the mutex.
The iobuffer now contains task 2's response.
5. Control returns to task 1 for it to check the iobuffer
response. It contains task 2's response, not its own.
Fix it by releasing the mutex only after the exclusive iobuffer
usage lifecycle is complete.
```
*Note: Please adhere to [Contributing
Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).*
## Summary
Fix a race on buffer. As the message above describes, there is contention
over a buffer from multiple tasks accessing a userfs. It can manifest as the
error message `userfs_rename: ERROR: Incorrect response: 19`. See below.
Should it be fixed with `goto` instead?
## Impact
No impact.
## Testing
`sim:userfs` defconfig with `CONFIG_DEBUG_FS_ERROR` to get the "Incorrect
response" error message. Full defconfig at the bottom. Use modified hello or
other app to demonstrate the issue.
For the demonstration, make the example userfs rename operation take some
time.
```diff
diff --git a/examples/userfs/userfs_main.c b/examples/userfs/userfs_main.c
index a2cf97ea5..6ad469a3b 100644
--- a/examples/userfs/userfs_main.c
+++ b/examples/userfs/userfs_main.c
@@ -519,6 +519,7 @@ static int ufstest_rmdir(FAR void *volinfo, FAR const
char *relpath)
static int ufstest_rename(FAR void *volinfo, FAR const char *oldrelpath,
FAR const char *newrelpath)
{
+ sleep(2);
FAR struct ufstest_file_s *file;
file = ufstest_findbyname(oldrelpath);
```
"Hello" program that demonstrates the race.
```c
#include <nuttx/config.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sched.h>
static int high_prio_stat(int argc, char *argv[])
{
int res;
struct stat statbuf;
/* wait for `main` to enter `rename` */
sleep(1);
/* will block waiting for mutex held by `rename` */
res = stat("/mnt/ufstest/File1", &statbuf);
if (res < 0)
{
return 1;
}
return 0;
}
int main(int argc, FAR char *argv[])
{
FAR char *nshargv[1];
int pid;
int res;
nshargv[0] = NULL;
pid = task_create("high_prio_stat", 120,
2048, high_prio_stat,
(FAR char * const *)nshargv);
if (pid < 0)
{
return 1;
}
/* take the mutex and block in userfs_main.c for 2 seconds */
res = rename("/mnt/ufstest/File3", "/mnt/ufstest/foobar");
if (res < 0)
{
return 1;
}
return 0;
}
```
It produces this result:
```
nsh> userfs
nsh> hello
userfs_rename: ERROR: Incorrect response: 19
nsh>
```
Full defconfig:
```
#
# This file is autogenerated: PLEASE DO NOT EDIT IT.
#
# You can use "make menuconfig" to make any modifications to the installed
.config file.
# You can then do "make savedefconfig" to generate a new defconfig file that
includes your
# modifications.
#
# CONFIG_DEBUG_INFO is not set
# CONFIG_NET_ETHERNET is not set
# CONFIG_NSH_CMDOPT_HEXDUMP is not set
# CONFIG_NSH_NETINIT is not set
CONFIG_ARCH="sim"
CONFIG_ARCH_BOARD="sim"
CONFIG_ARCH_BOARD_SIM=y
CONFIG_ARCH_CHIP="sim"
CONFIG_ARCH_SIM=y
CONFIG_BOARDCTL_POWEROFF=y
CONFIG_BOARD_LOOPSPERMSEC=0
CONFIG_BOOT_RUNFROMEXTSRAM=y
CONFIG_BUILTIN=y
CONFIG_CCACHE=y
CONFIG_DEBUG_ASSERTIONS=y
CONFIG_DEBUG_FEATURES=y
CONFIG_DEBUG_FS=y
CONFIG_DEBUG_FS_ERROR=y
CONFIG_DEBUG_FS_WARN=y
CONFIG_DEBUG_SYMBOLS=y
CONFIG_DEV_LOOP=y
CONFIG_ETC_FATDEVNO=2
CONFIG_ETC_ROMFS=y
CONFIG_ETC_ROMFSDEVNO=1
CONFIG_EXAMPLES_HELLO=y
CONFIG_EXAMPLES_USERFS=y
CONFIG_FAT_LCNAMES=y
CONFIG_FAT_LFN=y
CONFIG_FRAME_POINTER=y
CONFIG_FSUTILS_PASSWD=y
CONFIG_FSUTILS_PASSWD_READONLY=y
CONFIG_FS_FAT=y
CONFIG_FS_PROCFS=y
CONFIG_FS_ROMFS=y
CONFIG_FS_USERFS=y
CONFIG_IDLETHREAD_STACKSIZE=4096
CONFIG_INIT_ENTRYPOINT="nsh_main"
CONFIG_LIBC_ENVPATH=y
CONFIG_LIBC_EXECFUNCS=y
CONFIG_LIBC_MAX_EXITFUNS=1
CONFIG_MM_CUSTOMIZE_MANAGER=y
CONFIG_NET=y
CONFIG_NET_LOCAL=y
CONFIG_NET_LOOPBACK=y
CONFIG_NET_UDP=y
CONFIG_NSH_ARCHINIT=y
CONFIG_NSH_BUILTIN_APPS=y
CONFIG_NSH_FILE_APPS=y
CONFIG_NSH_READLINE=y
CONFIG_PATH_INITIAL="/bin"
CONFIG_READLINE_TABCOMPLETION=y
CONFIG_SCHED_HAVE_PARENT=y
CONFIG_SCHED_LPWORK=y
CONFIG_SCHED_WAITPID=y
CONFIG_SIM_ASAN=y
CONFIG_START_MONTH=6
CONFIG_START_YEAR=2008
CONFIG_SYSTEM_NSH=y
```
--
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]
