Skip to content

Commit

Permalink
fs: Fix the race condition in file_dup
Browse files Browse the repository at this point in the history
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
  • Loading branch information
xiaoxiang781216 committed Jan 3, 2021
1 parent 9f1c1c4 commit b1da811
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 19 deletions.
5 changes: 3 additions & 2 deletions fs/inode/fs_files.c
Expand Up @@ -256,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;
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion fs/inode/inode.h
Expand Up @@ -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
Expand Down
25 changes: 11 additions & 14 deletions fs/vfs/fs_dupfd.c
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion fs/vfs/fs_open.c
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion include/nuttx/fs/fs.h
Expand Up @@ -408,7 +408,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 */
Expand Down

0 comments on commit b1da811

Please sign in to comment.