Skip to content

Commit

Permalink
vfs/file: add reference counting to prevent accidental close during r…
Browse files Browse the repository at this point in the history 
…eading writing...

Signed-off-by: Shoukui Zhang <[email protected]>
  • Loading branch information
@Zhangshoukui @xiaoxiang781216
Zhangshoukui authored and xiaoxiang781216 committed Sep 17, 2024
1 parent f2fd0bc commit 4322312
Show file tree
Hide file tree
Showing 50 changed files with 335 additions and 146 deletions.
1 change: 1 addition & 0 deletions audio/audio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,7 @@ static int audio_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
{
audinfo("AUDIOIOC_UNREGISTERMQ\n");

fs_putfilep(upper->usermq);
upper->usermq = NULL;
ret = OK;
}
Expand Down
4 changes: 3 additions & 1 deletion drivers/net/telnet.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
{
FAR struct telnet_dev_s *priv;
FAR struct socket *psock;
FAR struct file *filep;
int ret;

/* Allocate instance data for this driver */
Expand Down Expand Up @@ -931,7 +932,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
* instance resided in the daemon's task group`).
*/

ret = sockfd_socket(session->ts_sd, &psock);
ret = sockfd_socket(session->ts_sd, &filep, &psock);
if (ret != OK)
{
nerr("ERROR: Failed to convert sd=%d to a socket structure\n",
Expand All @@ -940,6 +941,7 @@ static int telnet_session(FAR struct telnet_session_s *session)
}

ret = psock_dup2(psock, &priv->td_psock);
fs_putfilep(filep);
if (ret < 0)
{
nerr("ERROR: psock_dup2 failed: %d\n", ret);
Expand Down
42 changes: 24 additions & 18 deletions fs/aio/aioc_contain.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,35 +80,40 @@ FAR struct aio_container_s *aio_contain(FAR struct aiocb *aiocbp)
*/

aioc = aioc_alloc();
if (aioc != NULL)
if (aioc == NULL)
{
/* Initialize the container */
ret = -ENOMEM;
goto err_putfilep;
}

/* Initialize the container */

memset(aioc, 0, sizeof(struct aio_container_s));
aioc->aioc_aiocbp = aiocbp;
aioc->aioc_filep = filep;
aioc->aioc_pid = nxsched_getpid();
memset(aioc, 0, sizeof(struct aio_container_s));
aioc->aioc_aiocbp = aiocbp;
aioc->aioc_filep = filep;
aioc->aioc_pid = nxsched_getpid();

#ifdef CONFIG_PRIORITY_INHERITANCE
DEBUGVERIFY(nxsched_get_param (aioc->aioc_pid, &param));
aioc->aioc_prio = param.sched_priority;
DEBUGVERIFY(nxsched_get_param(aioc->aioc_pid, &param));
aioc->aioc_prio = param.sched_priority;
#endif

/* Add the container to the pending transfer list. */

ret = aio_lock();
if (ret < 0)
{
aioc_free(aioc);
goto errout;
}
/* Add the container to the pending transfer list. */

dq_addlast(&aioc->aioc_link, &g_aio_pending);
aio_unlock();
ret = aio_lock();
if (ret < 0)
{
aioc_free(aioc);
goto err_putfilep;
}

dq_addlast(&aioc->aioc_link, &g_aio_pending);
aio_unlock();

return aioc;

err_putfilep:
fs_putfilep(filep);
errout:
set_errno(-ret);
return NULL;
Expand Down Expand Up @@ -148,6 +153,7 @@ FAR struct aiocb *aioc_decant(FAR struct aio_container_s *aioc)
*/

aiocbp = aioc->aioc_aiocbp;
fs_putfilep(aioc->aioc_filep);
aioc_free(aioc);

aio_unlock();
Expand Down
123 changes: 107 additions & 16 deletions fs/inode/fs_files.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,33 @@
****************************************************************************/

static FAR struct file *files_fget_by_index(FAR struct filelist *list,
int l1, int l2)
int l1, int l2, FAR bool *new)
{
FAR struct file *filep;
irqstate_t flags;

flags = spin_lock_irqsave(NULL);

filep = &list->fl_files[l1][l2];
if (filep->f_inode != NULL)
{
filep->f_refs++;
}
else if (new == NULL)
{
filep = NULL;
}
else if (filep->f_refs)
{
filep->f_refs++;
}
else
{
filep->f_refs = 2;
*new = true;
}

spin_unlock_irqrestore(NULL, flags);

return filep;
}

Expand Down Expand Up @@ -180,10 +196,11 @@ static void task_fssync(FAR struct tcb_s *tcb, FAR void *arg)
{
FAR struct file *filep;

filep = files_fget_by_index(list, i, j);
if (filep->f_inode != NULL)
filep = files_fget_by_index(list, i, j, NULL);
if (filep != NULL)
{
file_fsync(filep);
fs_putfilep(filep);
}
}
}
Expand Down Expand Up @@ -215,13 +232,15 @@ static int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2,
int flags)
{
FAR struct filelist *list;
FAR struct file *filep1;
FAR struct file *filep;
#ifdef CONFIG_FDCHECK
uint8_t f_tag_fdcheck;
#endif
#ifdef CONFIG_FDSAN
uint64_t f_tag_fdsan;
#endif
bool new = false;
int count;
int ret;

Expand Down Expand Up @@ -254,12 +273,17 @@ static int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2,
}
}

filep = files_fget(list, fd2);
if (filep == NULL)
filep1 = files_fget(list, fd1);
if (filep1 == NULL)
{
return -EBADF;
}

filep = files_fget_by_index(list,
fd2 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK,
fd2 % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK,
&new);

#ifdef CONFIG_FDSAN
f_tag_fdsan = filep->f_tag_fdsan;
#endif
Expand All @@ -270,9 +294,16 @@ static int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2,

/* Perform the dup3 operation */

ret = file_dup3(files_fget(list, fd1), filep, flags);
ret = file_dup3(filep1, filep, flags);
fs_putfilep(filep1);
fs_putfilep(filep);
if (ret < 0)
{
if (new)
{
fs_putfilep(filep);
}

return ret;
}

Expand Down Expand Up @@ -482,7 +513,7 @@ int files_countlist(FAR struct filelist *list)
FAR struct file *files_fget(FAR struct filelist *list, int fd)
{
return files_fget_by_index(list, fd / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK,
fd % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK);
fd % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK, NULL);
}

/****************************************************************************
Expand Down Expand Up @@ -541,6 +572,7 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode,
filep->f_pos = pos;
filep->f_inode = inode;
filep->f_priv = priv;
filep->f_refs = 1;

goto found;
}
Expand Down Expand Up @@ -607,7 +639,9 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist,
{
for (j = 0; j < CONFIG_NFILE_DESCRIPTORS_PER_BLOCK; j++)
{
FAR struct file *filep2;
FAR struct file *filep;
bool new = false;

fd = i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j;
#ifdef CONFIG_FDCLONE_STDIO
Expand All @@ -625,8 +659,8 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist,
}
#endif

filep = files_fget_by_index(plist, i, j);
if (filep->f_inode == NULL)
filep = files_fget_by_index(plist, i, j, NULL);
if (filep == NULL)
{
continue;
}
Expand All @@ -642,25 +676,36 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist,
#endif
if (!spawn_file_is_duplicateable(actions, fd, fcloexec))
{
fs_putfilep(filep);
continue;
}
}
else if (fcloexec)
{
fs_putfilep(filep);
continue;
}

ret = files_extend(clist, i + 1);
if (ret < 0)
{
fs_putfilep(filep);
return ret;
}

/* Yes... duplicate it for the child, include O_CLOEXEC flag. */

ret = file_dup2(filep, files_fget_by_index(clist, i, j));
filep2 = files_fget_by_index(clist, i, j, &new);
ret = file_dup2(filep, filep2);
fs_putfilep(filep2);
fs_putfilep(filep);
if (ret < 0)
{
if (new)
{
fs_putfilep(filep2);
}

return ret;
}
}
Expand Down Expand Up @@ -721,17 +766,55 @@ int fs_getfilep(int fd, FAR struct file **filep)

*filep = files_fget(list, fd);

/* if f_inode is NULL, fd was closed */
/* if *filep is NULL, fd was closed */

if ((*filep)->f_inode == NULL)
if (*filep == NULL)
{
*filep = NULL;
return -EBADF;
}

return OK;
}

/****************************************************************************
* Name: fs_putfilep
*
* Description:
* Handles reference counts for files, less than or equal to 0 and close
* the file
*
* Input Parameters:
* filep - The caller provided location in which to return the 'struct
* file' instance.
****************************************************************************/

int fs_putfilep(FAR struct file *filep)
{
irqstate_t flags;
int ret = 0;
int refs;

DEBUGASSERT(filep);
flags = spin_lock_irqsave(NULL);

refs = --filep->f_refs;

spin_unlock_irqrestore(NULL, flags);

/* If refs is zero, the close() had called, closing it now. */

if (refs == 0)
{
ret = file_close(filep);
if (ret < 0)
{
ferr("ERROR: fs putfilep file_close() failed: %d\n", ret);
}
}

return ret;
}

/****************************************************************************
* Name: nx_dup2_from_tcb
*
Expand Down Expand Up @@ -870,12 +953,20 @@ int nx_close_from_tcb(FAR struct tcb_s *tcb, int fd)

/* If the file was properly opened, there should be an inode assigned */

if (filep->f_inode == NULL)
if (filep == NULL)
{
return -EBADF;
}

return file_close(filep);
/* files_fget will increase the reference count, there call fs_putfilep
* reduce reference count.
*/

fs_putfilep(filep);

/* Undo the last reference count from file_allocate_from_tcb */

return fs_putfilep(filep);
}

/****************************************************************************
Expand Down
5 changes: 5 additions & 0 deletions fs/mmap/fs_mmap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,11 @@ FAR void *mmap(FAR void *start, size_t length, int prot, int flags,

ret = file_mmap_(filep, start, length,
prot, flags, offset, false, &mapped);
if (fd != -1)
{
fs_putfilep(filep);
}

if (ret < 0)
{
goto errout;
Expand Down
Loading

0 comments on commit 4322312

Please sign in to comment.