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 <dongjiuzhu1@xiaomi.com>

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
  *

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;
 }
 

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;
 }

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;
 }
 

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
  *