From 86fab49c46665c4410a19374f89cb808624345e3 Mon Sep 17 00:00:00 2001
From: Xiang Xiao <xiaoxiang@xiaomi.com>
Date: Sat, 2 Jan 2021 22:08:43 +0800
Subject: [PATCH] fs: Fix the race condition in file_dup

NULL inode passed to files_allocate doesn't mark file struct in the
allocated state, so other threads which invovle in file allocation
(e.g. open or dup) may allocate the same file struct again.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: I53ff876eae3c7a1e311e7f671686b73a4b4ef891
---
 fs/inode/fs_files.c   |  5 +++--
 fs/inode/inode.h      |  2 +-
 fs/mqueue/mq_open.c   |  2 +-
 fs/vfs/fs_dupfd.c     | 31 ++++++++++++++-----------------
 fs/vfs/fs_open.c      |  2 +-
 include/nuttx/fs/fs.h |  2 +-
 6 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c
index be63be8770..e28685f21c 100644
--- a/fs/inode/fs_files.c
+++ b/fs/inode/fs_files.c
@@ -256,7 +256,8 @@ errout_with_sem:
  *
  ****************************************************************************/
 
-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;
@@ -282,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;
         }
diff --git a/fs/inode/inode.h b/fs/inode/inode.h
index 62a739f59b..dea5753491 100644
--- a/fs/inode/inode.h
+++ b/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
diff --git a/fs/mqueue/mq_open.c b/fs/mqueue/mq_open.c
index ab703df84d..86901e83ac 100644
--- a/fs/mqueue/mq_open.c
+++ b/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);
diff --git a/fs/vfs/fs_dupfd.c b/fs/vfs/fs_dupfd.c
index 15ed0af9d9..fdf73d94cb 100644
--- a/fs/vfs/fs_dupfd.c
+++ b/fs/vfs/fs_dupfd.c
@@ -51,32 +51,29 @@
 
 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);
+  memset(&filep2, 0, sizeof(filep2));
+  ret = file_dup2(filep, &filep2);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  /* 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)
     {
+      file_close(&filep2);
       return fd2;
     }
 
-  ret = fs_getfilep(fd2, &filep2);
-  if (ret < 0)
-    {
-      files_release(fd2);
-      return ret;
-    }
-
-  ret = file_dup2(filep, filep2);
-  if (ret < 0)
-    {
-      files_release(fd2);
-      return ret;
-    }
-
   return fd2;
 }
 
diff --git a/fs/vfs/fs_open.c b/fs/vfs/fs_open.c
index 0fcc78dfec..5ded1acedd 100644
--- a/fs/vfs/fs_open.c
+++ b/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;
diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h
index 29636d4060..4005e3a75a 100644
--- a/include/nuttx/fs/fs.h
+++ b/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 */