fs: Fix the race condition in file_dup

fs/inode/fs_files.c

@@ -137,6 +137,7 @@ int file_dup2(FAR struct file *filep1, FAR struct file *filep2)
 {
   FAR struct filelist *list;
   FAR struct inode *inode;
+  struct file temp;
   int ret;
 
   if (filep1 == NULL || filep1->f_inode == NULL || filep2 == NULL)
@@ -169,18 +170,6 @@ int file_dup2(FAR struct file *filep1, FAR struct file *filep2)
         }
     }
 
-  /* If there is already an inode contained in the new file structure,
-   * close the file and release the inode.
-   */
-
-  ret = file_close(filep2);
-  if (ret < 0)
-    {
-      /* An error occurred while closing the driver */
-
-      goto errout_with_sem;
-    }
-
   /* Increment the reference count on the contained inode */
 
   inode = filep1->f_inode;
@@ -192,9 +181,10 @@ int file_dup2(FAR struct file *filep1, FAR struct file *filep2)
 
   /* Then clone the file structure */
 
-  filep2->f_oflags = filep1->f_oflags;
-  filep2->f_pos    = filep1->f_pos;
-  filep2->f_inode  = inode;
+  temp.f_oflags = filep1->f_oflags;
+  temp.f_pos    = filep1->f_pos;
+  temp.f_inode  = inode;
+  temp.f_priv   = filep1->f_priv;
 
   /* Call the open method on the file, driver, mountpoint so that it
    * can maintain the correct open counts.
@@ -207,14 +197,14 @@ int file_dup2(FAR struct file *filep1, FAR struct file *filep2)
         {
           /* Dup the open file on the in the new file structure */
 
-          ret = inode->u.i_mops->dup(filep1, filep2);
+          ret = inode->u.i_mops->dup(filep1, &temp);
         }
       else
 #endif
         {
           /* (Re-)open the pseudo file or device driver */
 
-          ret = inode->u.i_ops->open(filep2);
+          ret = inode->u.i_ops->open(&temp);
         }
 
       /* Handle open failures */
@@ -225,6 +215,17 @@ int file_dup2(FAR struct file *filep1, FAR struct file *filep2)
         }
     }
 
+  /* If there is already an inode contained in the new file structure,
+   * close the file and release the inode.
+   */
+
+  ret = file_close(filep2);
+  DEBUGASSERT(ret == 0);
+
+  /* Return the file structure */
+
+  memcpy(filep2, &temp, sizeof(temp));
+
   if (list != NULL)
     {
       _files_semgive(list);
@@ -235,11 +236,7 @@ int file_dup2(FAR struct file *filep1, FAR struct file *filep2)
   /* Handle various error conditions */
 
 errout_with_inode:
-
-  inode_release(filep2->f_inode);
-  filep2->f_oflags = 0;
-  filep2->f_pos    = 0;
-  filep2->f_inode  = NULL;
+  inode_release(inode);
 
 errout_with_sem:
   if (list != NULL)
@@ -259,7 +256,8 @@ int file_dup2(FAR struct file *filep1, FAR struct file *filep2)
  *
  ****************************************************************************/
 
-int files_allocate(FAR struct inode *inode, int oflags, off_t pos, int minfd)
+int files_allocate(FAR struct inode *inode, int oflags, off_t pos,
+                   FAR void *priv, int minfd)
 {
   FAR struct filelist *list;
   int ret;
@@ -285,7 +283,7 @@ int files_allocate(FAR struct inode *inode, int oflags, off_t pos, int minfd)
           list->fl_files[i].f_oflags = oflags;
           list->fl_files[i].f_pos    = pos;
           list->fl_files[i].f_inode  = inode;
-          list->fl_files[i].f_priv   = NULL;
+          list->fl_files[i].f_priv   = priv;
           _files_semgive(list);
           return i;
         }

fs/inode/inode.h

@@ -387,7 +387,7 @@ void weak_function files_initialize(void);
  ****************************************************************************/
 
 int files_allocate(FAR struct inode *inode, int oflags, off_t pos,
-                   int minfd);
+                   FAR void *priv, int minfd);
 
 /****************************************************************************
  * Name: files_close

fs/mqueue/mq_open.c

@@ -269,7 +269,7 @@ static mqd_t nxmq_vopen(FAR const char *mq_name, int oflags, va_list ap)
       return ret;
     }
 
-  ret = files_allocate(mq.f_inode, mq.f_oflags, mq.f_pos, 0);
+  ret = files_allocate(mq.f_inode, mq.f_oflags, mq.f_pos, mq.f_priv, 0);
   if (ret < 0)
     {
       file_mq_close(&mq);

fs/vfs/fs_dupfd.c

@@ -51,30 +51,27 @@
 
 int file_dup(FAR struct file *filep, int minfd)
 {
-  FAR struct file *filep2;
+  struct file filep2;
   int fd2;
   int ret;
 
-  /* Allocate a new file descriptor for the inode */
+  /* Let file_dup2() do the real work */
 
-  fd2 = files_allocate(NULL, 0, 0, minfd);
-  if (fd2 < 0)
-    {
-      return fd2;
-    }
-
-  ret = fs_getfilep(fd2, &filep2);
+  memset(&filep2, 0, sizeof(filep2));
+  ret = file_dup2(filep, &filep2);
   if (ret < 0)
     {
-      files_release(fd2);
       return ret;
     }
 
-  ret = file_dup2(filep, filep2);
-  if (ret < 0)
+  /* Then allocate a new file descriptor for the inode */
+
+  fd2 = files_allocate(filep2.f_inode, filep2.f_oflags,
+                       filep2.f_pos, filep2.f_priv, minfd);
+  if (fd2 < 0)
     {
-      files_release(fd2);
-      return ret;
+      file_close(&filep2);
+      return fd2;
     }
 
   return fd2;

fs/vfs/fs_open.c

@@ -204,7 +204,7 @@ int nx_vopen(FAR const char *path, int oflags, va_list ap)
 
   /* Associate the inode with a file structure */
 
-  fd = files_allocate(inode, oflags, 0, 0);
+  fd = files_allocate(inode, oflags, 0, NULL, 0);
   if (fd < 0)
     {
       ret = fd;

include/nuttx/fs/fs.h

@@ -401,7 +401,7 @@ struct file
   int               f_oflags;   /* Open mode flags */
   off_t             f_pos;      /* File position */
   FAR struct inode *f_inode;    /* Driver or file system interface */
-  void             *f_priv;     /* Per file driver private data */
+  FAR void         *f_priv;     /* Per file driver private data */
 };
 
 /* This defines a list of files indexed by the file descriptor */