This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 148f0ce7dd0 fs/inode: use file_allocate,file_dup to avoid
racecondition to allocate fd
148f0ce7dd0 is described below
commit 148f0ce7dd0fcf01547899ce0591234a2346edb4
Author: dongjiuzhu1 <[email protected]>
AuthorDate: Sat Sep 6 15:21:58 2025 +0800
fs/inode: use file_allocate,file_dup to avoid racecondition to allocate fd
issue description:
task A: NSH:
1.open->
reboot->sync->task_fsfsync
2.nx_vopen-> context switch
3.fdlist_allocate: ---->
4.fsync->file_sync->assert(inode or priv is empty)
(new fd with empty filep)
5.file_vopen:
(init empty filep)
6.return fd
Task A allocates a new fd with an empty filep in fdlist_allocate. Before
it can fully initialize the filep in file_vopen, the NSH task triggers a
file - system sync operation. The sync operation encounters the empty
filep associated with the newly allocated fd, causing the assertion to
fail and the system to crash.
To resolve this race condition, we should modify the fd allocation
process. Instead of allocating a new fd with an empty filep first and
then initializing it later, we should use the file_allocate_from_inode
function. This function allows us to initialize the file structure first
and then bind it to the new filep when allocating the fd. By doing so,
we ensure that the filep is always properly initialized before it is
used in any file - system operations, thus preventing the assertion
failure and the subsequent system crash.
Signed-off-by: dongjiuzhu1 <[email protected]>
---
fs/inode/fs_files.c | 115 +++++++++++++++-----------------------------------
fs/mqueue/mq_open.c | 16 ++++---
fs/shm/shm_open.c | 18 +++++---
fs/vfs/fs_open.c | 20 +++++----
include/nuttx/fs/fs.h | 73 +++++++++-----------------------
5 files changed, 90 insertions(+), 152 deletions(-)
diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c
index 2645a40fe31..8ecb9cfd19f 100644
--- a/fs/inode/fs_files.c
+++ b/fs/inode/fs_files.c
@@ -632,81 +632,6 @@ found:
#endif
}
-/****************************************************************************
- * Name: fdlist_allocate
- *
- * Description:
- * Allocate a struct fd instance and associate it with an empty file
- * instance. The difference between this function and
- * file_allocate_from_inode is that this function is only used for
- * placeholder purposes. Later, the caller will initialize the file entity
- * through the returned filep.
- *
- * The fd allocated by this function can be released using fdlist_close.
- *
- * After the function call is completed, it will hold a reference count
- * for the filep. Therefore, when the filep is no longer in use, it is
- * necessary to call file_put to release the reference count, in order
- * to avoid a race condition where the file might be closed during
- * this process.
- *
- * Returned Value:
- * Returns the file descriptor == index into the files array on success;
- * a negated errno value is returned on any failure.
- *
- ****************************************************************************/
-
-int fdlist_allocate(FAR struct fdlist *list, int oflags,
- int minfd, FAR struct file **filep)
-{
- int fd;
-
- *filep = fs_heap_zalloc(sizeof(struct file));
- if (*filep == NULL)
- {
- return -ENOMEM;
- }
-
- file_ref(*filep);
- fd = fdlist_dupfile(list, oflags, minfd, *filep);
- if (fd < 0)
- {
- file_put(*filep);
- }
-
- return fd;
-}
-
-/****************************************************************************
- * Name: file_allocate
- *
- * Description:
- * Allocate a struct fd instance and associate it with an empty file
- * instance. The difference between this function and
- * file_allocate_from_inode is that this function is only used for
- * placeholder purposes. Later, the caller will initialize the file entity
- * through the returned filep.
- *
- * The fd allocated by this function can be released using nx_close.
- *
- * After the function call is completed, it will hold a reference count
- * for the filep. Therefore, when the filep is no longer in use, it is
- * necessary to call file_put to release the reference count, in order
- * to avoid a race condition where the file might be closed during
- * this process.
- *
- * Returned Value:
- * Returns the file descriptor == index into the files array on success;
- * a negated errno value is returned on any failure.
- *
- ****************************************************************************/
-
-int file_allocate(int oflags, int minfd, FAR struct file **filep)
-{
- return fdlist_allocate(nxsched_get_fdlist_from_tcb(this_task()),
- oflags, minfd, filep);
-}
-
/****************************************************************************
* Name: file_allocate_from_inode
*
@@ -726,10 +651,10 @@ int file_allocate_from_inode(FAR struct inode *inode, int
oflags, off_t pos,
FAR struct file *filep;
int fd;
- fd = file_allocate(oflags, minfd, &filep);
- if (fd < 0)
+ filep = file_allocate();
+ if (filep == NULL)
{
- return fd;
+ return -ENOMEM;
}
inode_addref(inode);
@@ -740,8 +665,12 @@ int file_allocate_from_inode(FAR struct inode *inode, int
oflags, off_t pos,
#if CONFIG_FS_LOCK_BUCKET_SIZE > 0
filep->f_locked = false;
#endif
-
- file_put(filep);
+ fd = file_dup(filep, minfd, oflags);
+ if (fd < 0)
+ {
+ inode_release(inode);
+ file_deallocate(filep);
+ }
return fd;
}
@@ -839,6 +768,32 @@ int fdlist_copy(FAR struct fdlist *plist, FAR struct
fdlist *clist,
return OK;
}
+/****************************************************************************
+ * Name: file_allocate
+ *
+ * Description:
+ * Allocate a file instance and return
+ *
+ ****************************************************************************/
+
+FAR struct file *file_allocate(void)
+{
+ return fs_heap_zalloc(sizeof(struct file));
+}
+
+/****************************************************************************
+ * Name: file_deallocate
+ *
+ * Description:
+ * Free a file instance.
+ *
+ ****************************************************************************/
+
+void file_deallocate(FAR struct file *filep)
+{
+ fs_heap_free(filep);
+}
+
/****************************************************************************
* Name: file_get2
*
diff --git a/fs/mqueue/mq_open.c b/fs/mqueue/mq_open.c
index 909c4dabf9f..514f053997e 100644
--- a/fs/mqueue/mq_open.c
+++ b/fs/mqueue/mq_open.c
@@ -350,20 +350,26 @@ static mqd_t nxmq_vopen(FAR const char *mq_name, int
oflags, va_list ap)
int ret;
int fd;
- fd = file_allocate(oflags, 0, &mq);
- if (fd < 0)
+ mq = file_allocate();
+ if (mq == NULL)
{
- return fd;
+ return -ENOMEM;
}
ret = file_mq_vopen(mq, mq_name, oflags, getumask(), ap, &created);
- file_put(mq);
if (ret < 0)
{
- nx_close(fd);
+ file_deallocate(mq);
return ret;
}
+ fd = file_dup(mq, 0, oflags);
+ if (fd < 0)
+ {
+ file_close(mq);
+ file_deallocate(mq);
+ }
+
return fd;
}
diff --git a/fs/shm/shm_open.c b/fs/shm/shm_open.c
index 7d799743c9b..b83b324c6fc 100644
--- a/fs/shm/shm_open.c
+++ b/fs/shm/shm_open.c
@@ -176,21 +176,29 @@ int shm_open(FAR const char *name, int oflag, mode_t mode)
int ret;
int fd;
- fd = file_allocate(oflag | O_CLOEXEC, 0, &shm);
- if (fd < 0)
+ shm = file_allocate();
+ if (shm == NULL)
{
- set_errno(-fd);
+ set_errno(ENOMEM);
return ERROR;
}
ret = file_shm_open(shm, name, oflag, mode);
- file_put(shm);
if (ret < 0)
{
- nx_close(fd);
+ file_deallocate(shm);
set_errno(-ret);
return ERROR;
}
+ fd = file_dup(shm, 0, oflag | O_CLOEXEC);
+ if (fd < 0)
+ {
+ file_close(shm);
+ file_deallocate(shm);
+ set_errno(-fd);
+ return ERROR;
+ }
+
return fd;
}
diff --git a/fs/vfs/fs_open.c b/fs/vfs/fs_open.c
index bab898463bf..f85c1eec24a 100644
--- a/fs/vfs/fs_open.c
+++ b/fs/vfs/fs_open.c
@@ -315,24 +315,26 @@ static int nx_vopen(FAR struct fdlist *list,
int ret;
int fd;
- /* Allocate a new file descriptor for the inode */
-
- fd = fdlist_allocate(list, oflags, 0, &filep);
- if (fd < 0)
+ filep = file_allocate();
+ if (filep == NULL)
{
- return fd;
+ return -ENOMEM;
}
- /* Let file_vopen() do all of the work */
-
ret = file_vopen(filep, path, oflags, getumask(), ap);
- file_put(filep);
if (ret < 0)
{
- fdlist_close(list, fd);
+ file_deallocate(filep);
return ret;
}
+ fd = fdlist_dupfile(list, oflags, 0, filep);
+ if (fd < 0)
+ {
+ file_close(filep);
+ file_deallocate(filep);
+ }
+
return fd;
}
diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h
index c819d464b8d..4283e86ea5b 100644
--- a/include/nuttx/fs/fs.h
+++ b/include/nuttx/fs/fs.h
@@ -1035,59 +1035,6 @@ int fdlist_get2(FAR struct fdlist *list, int fd,
int fdlist_dupfile(FAR struct fdlist *list, int oflags, int minfd,
FAR struct file *filep);
-/****************************************************************************
- * Name: fdlist_allocate
- *
- * Description:
- * Allocate a struct fd instance and associate it with an empty file
- * instance. The difference between this function and
- * file_allocate_from_inode is that this function is only used for
- * placeholder purposes. Later, the caller will initialize the file entity
- * through the returned filep.
- *
- * The fd allocated by this function can be released using fdlist_close.
- *
- * After the function call is completed, it will hold a reference count
- * for the filep. Therefore, when the filep is no longer in use, it is
- * necessary to call file_put to release the reference count, in order
- * to avoid a race condition where the file might be closed during
- * this process.
- *
- * Returned Value:
- * Returns the file descriptor == index into the files array on success;
- * a negated errno value is returned on any failure.
- *
- ****************************************************************************/
-
-int fdlist_allocate(FAR struct fdlist *list, int oflags,
- int minfd, FAR struct file **filep);
-
-/****************************************************************************
- * Name: file_allocate
- *
- * Description:
- * Allocate a struct fd instance and associate it with an empty file
- * instance. The difference between this function and
- * file_allocate_from_inode is that this function is only used for
- * placeholder purposes. Later, the caller will initialize the file entity
- * through the returned filep.
- *
- * The fd allocated by this function can be released using nx_close.
- *
- * After the function call is completed, it will hold a reference count
- * for the filep. Therefore, when the filep is no longer in use, it is
- * necessary to call file_put to release the reference count, in order
- * to avoid a race condition where the file might be closed during
- * this process.
- *
- * Returned Value:
- * Returns the file descriptor == index into the files array on success;
- * a negated errno value is returned on any failure.
- *
- ****************************************************************************/
-
-int file_allocate(int oflags, int minfd, FAR struct file **filep);
-
/****************************************************************************
* Name: file_allocate_from_inode
*
@@ -1264,6 +1211,26 @@ int fdlist_open(FAR struct fdlist *list,
int nx_open(FAR const char *path, int oflags, ...);
+/****************************************************************************
+ * Name: file_allocate
+ *
+ * Description:
+ * Allocate a file instance and return
+ *
+ ****************************************************************************/
+
+FAR struct file *file_allocate(void);
+
+/****************************************************************************
+ * Name: file_deallocate
+ *
+ * Description:
+ * Free a file instance.
+ *
+ ****************************************************************************/
+
+void file_deallocate(FAR struct file *filep);
+
/****************************************************************************
* Name: file_get2
*
