OrgApache/incubator-nuttx
OrgApache
/
incubator-nuttx
mirror of https://github.com/apache/incubator-nuttx.git
1
0
Fork 0
Code Issues Releases Wiki Activity

fs/unionfs: remove excessive protection to avoid deadlock 

Deadlock during recursive access if unionfs overlays procfs,
check the critical segment only and remove the useless protection part.

|#0  unionfs_statfs (mountpt=0xf3df4540, buf=0xf3de2f0c) at unionfs/fs_unionfs.c:2136
...
|#6  0x08069429 in procfs_read (filep=0xf3df4574, buffer=0xf3df4610 "...", buflen=1024) at procfs/fs_procfs.c:412
|#7  0x0806c339 in unionfs_read (filep=0xf3de219c, buffer=0xf3df4610 "...", buflen=1024) at unionfs/fs_unionfs.c:1026

original call stack:
(gdb) bt
|#0  unionfs_statfs (mountpt=0xf3df4540, buf=0xf3de2f0c) at unionfs/fs_unionfs.c:2136
|#1  0x08071629 in mountpoint_filter (node=0xf3df4540, dirpath=0xf3df4a28 "/proc", arg=0xf3de2fc4) at mount/fs_foreachmountpoint.c:119
|#2  0x0807171b in foreach_inodelevel (node=0xf3df4540, info=0xf3df4a20) at inode/fs_foreachinode.c:90
|#3  0x08071898 in foreach_inode (handler=0x8071530 <mountpoint_filter>, arg=0xf3de2fc4) at inode/fs_foreachinode.c:193
|#4  0x080716c1 in foreach_mountpoint (handler=0x8070e2f <blocks_entry>, arg=0xf3de300c) at mount/fs_foreachmountpoint.c:169
|#5  0x08071399 in mount_read (filep=0xf3df4574, buffer=0xf3df4610 "...", buflen=1024) at mount/fs_procfs_mount.c:537
|#6  0x08069429 in procfs_read (filep=0xf3df4574, buffer=0xf3df4610 "...", buflen=1024) at procfs/fs_procfs.c:412
|#7  0x0806c339 in unionfs_read (filep=0xf3de219c, buffer=0xf3df4610 "...", buflen=1024) at unionfs/fs_unionfs.c:1026
|#8  0x080657a2 in file_read (filep=0xf3de219c, buf=0xf3df4610, nbytes=1024) at vfs/fs_read.c:110
|#9  0x0806581a in nx_read (fd=3, buf=0xf3df4610, nbytes=1024) at vfs/fs_read.c:175
|#10 0x08065847 in read (fd=3, buf=0xf3df4610, nbytes=1024) at vfs/fs_read.c:206
|#11 0x0805a242 in nsh_catfile (vtbl=0xf3df3f10, cmd=0xf3df4378 "df", filepath=0x808d5ed "/proc/fs/blocks") at nsh_fsutils.c:116
|#12 0x0805b1de in cmd_df (vtbl=0xf3df3f10, argc=1, argv=0xf3de32c0) at nsh_mntcmds.c:73
|#13 0x08056370 in nsh_command (vtbl=0xf3df3f10, argc=1, argv=0xf3de32c0) at nsh_command.c:1061
|#14 0x08053b16 in nsh_execute (vtbl=0xf3df3f10, argc=1, argv=0xf3de32c0, redirfile=0x0, oflags=0) at nsh_parse.c:741
|#15 0x08055998 in nsh_parse_command (vtbl=0xf3df3f10, cmdline=0xf3df4378 "df") at nsh_parse.c:2578
|#16 0x08055a7b in nsh_parse (vtbl=0xf3df3f10, cmdline=0xf3df4378 "df") at nsh_parse.c:2662
|#17 0x0805d691 in nsh_session (pstate=0xf3df3f10, login=1 '\001', argc=1, argv=0xf3de34b0) at nsh_session.c:191
|#18 0x0805b542 in nsh_consolemain (argc=1, argv=0xf3de34b0) at nsh_consolemain.c:115
|#19 0x0805346c in nsh_main (argc=1, argv=0xf3de34b0) at nsh_main.c:168
|#20 0x0805075a in nxtask_startup (entrypt=0x805340a <nsh_main>, argc=1, argv=0xf3de34b0) at sched/task_startup.c:165
|#21 0x08049713 in nxtask_start () at task/task_start.c:144
|#22 0x00000000 in ?? ()

Change-Id: Ic4c7aff0ea50388a371c525745e817a787dabcca
Signed-off-by: chao.an <anchao@xiaomi.com>
This commit is contained in:
chao.an 2020-12-14 14:59:17 +08:00 committed by Xiang Xiao
parent cb71469f85
commit 32bf92c5c3
1 changed files with 36 additions and 236 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

272
fs/unionfs/fs_unionfs.c
View File

@ -979,7 +979,6 @@ static ssize_t unionfs_read(FAR struct file *filep, FAR char *buffer,
FAR struct unionfs_file_s *uf;
FAR struct unionfs_mountpt_s *um;
FAR const struct mountpt_operations *ops;
int ret = -EPERM;
finfo("buflen: %lu\n", (unsigned long)buflen);
@ -988,14 +987,6 @@ static ssize_t unionfs_read(FAR struct file *filep, FAR char *buffer,
DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
ui = (FAR struct unionfs_inode_s *)filep->f_inode->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(ui != NULL && filep->f_priv != NULL);
uf = (FAR struct unionfs_file_s *)filep->f_priv;
@ -1008,13 +999,7 @@ static ssize_t unionfs_read(FAR struct file *filep, FAR char *buffer,
/* Perform the lower level read operation */
if (ops->read != NULL)
{
ret = ops->read(&uf->uf_file, buffer, buflen);
}
unionfs_semgive(ui);
return ret;
return ops->read ? ops->read(&uf->uf_file, buffer, buflen) : -EPERM;
}
/****************************************************************************
@ -1028,7 +1013,6 @@ static ssize_t unionfs_write(FAR struct file *filep, FAR const char *buffer,
FAR struct unionfs_file_s *uf;
FAR struct unionfs_mountpt_s *um;
FAR const struct mountpt_operations *ops;
int ret = -EPERM;
finfo("buflen: %lu\n", (unsigned long)buflen);
@ -1037,14 +1021,6 @@ static ssize_t unionfs_write(FAR struct file *filep, FAR const char *buffer,
DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
ui = (FAR struct unionfs_inode_s *)filep->f_inode->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(ui != NULL && filep->f_priv != NULL);
uf = (FAR struct unionfs_file_s *)filep->f_priv;
@ -1057,13 +1033,7 @@ static ssize_t unionfs_write(FAR struct file *filep, FAR const char *buffer,
/* Perform the lower level write operation */
if (ops->write != NULL)
{
ret = ops->write(&uf->uf_file, buffer, buflen);
}
unionfs_semgive(ui);
return ret;
return ops->write ? ops->write(&uf->uf_file, buffer, buflen) : -EPERM;
}
/****************************************************************************
@ -1076,7 +1046,6 @@ static off_t unionfs_seek(FAR struct file *filep, off_t offset, int whence)
FAR struct unionfs_file_s *uf;
FAR struct unionfs_mountpt_s *um;
FAR const struct mountpt_operations *ops;
int ret;
finfo("offset: %lu whence: %d\n", (unsigned long)offset, whence);
@ -1085,14 +1054,6 @@ static off_t unionfs_seek(FAR struct file *filep, off_t offset, int whence)
DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
ui = (FAR struct unionfs_inode_s *)filep->f_inode->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(ui != NULL && filep->f_priv != NULL);
uf = (FAR struct unionfs_file_s *)filep->f_priv;
@ -1111,6 +1072,16 @@ static off_t unionfs_seek(FAR struct file *filep, off_t offset, int whence)
}
else
{
int ret;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
/* No... Just set the common file position value */
switch (whence)
@ -1137,9 +1108,10 @@ static off_t unionfs_seek(FAR struct file *filep, off_t offset, int whence)
offset = (off_t)-EINVAL;
break;
}
unionfs_semgive(ui);
}
unionfs_semgive(ui);
return offset;
}
@ -1153,7 +1125,6 @@ static int unionfs_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
FAR struct unionfs_file_s *uf;
FAR struct unionfs_mountpt_s *um;
FAR const struct mountpt_operations *ops;
int ret = -ENOTTY;
finfo("cmd: %d arg: %lu\n", cmd, arg);
@ -1162,14 +1133,6 @@ static int unionfs_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
ui = (FAR struct unionfs_inode_s *)filep->f_inode->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(ui != NULL && filep->f_priv != NULL);
uf = (FAR struct unionfs_file_s *)filep->f_priv;
@ -1182,13 +1145,7 @@ static int unionfs_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
/* Perform the lower level ioctl operation */
if (ops->ioctl != NULL)
{
ret = ops->ioctl(&uf->uf_file, cmd, arg);
}
unionfs_semgive(ui);
return ret;
return ops->ioctl ? ops->ioctl(&uf->uf_file, cmd, arg) : -ENOTTY;
}
/****************************************************************************
@ -1201,7 +1158,6 @@ static int unionfs_sync(FAR struct file *filep)
FAR struct unionfs_file_s *uf;
FAR struct unionfs_mountpt_s *um;
FAR const struct mountpt_operations *ops;
int ret = -EINVAL;
finfo("filep=%p\n", filep);
@ -1210,14 +1166,6 @@ static int unionfs_sync(FAR struct file *filep)
DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
ui = (FAR struct unionfs_inode_s *)filep->f_inode->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(ui != NULL && filep->f_priv != NULL);
uf = (FAR struct unionfs_file_s *)filep->f_priv;
@ -1230,13 +1178,7 @@ static int unionfs_sync(FAR struct file *filep)
/* Perform the lower level sync operation */
if (ops->sync != NULL)
{
ret = ops->sync(&uf->uf_file);
}
unionfs_semgive(ui);
return ret;
return ops->sync ? ops->sync(&uf->uf_file) : -EINVAL;
}
/****************************************************************************
@ -1259,14 +1201,6 @@ static int unionfs_dup(FAR const struct file *oldp, FAR struct file *newp)
DEBUGASSERT(oldp != NULL && oldp->f_inode != NULL);
ui = (FAR struct unionfs_inode_s *)oldp->f_inode->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(ui != NULL && oldp->f_priv != NULL);
oldpriv = (FAR struct unionfs_file_s *)oldp->f_priv;
@ -1326,7 +1260,6 @@ static int unionfs_fstat(FAR const struct file *filep, FAR struct stat *buf)
FAR struct unionfs_file_s *uf;
FAR struct unionfs_mountpt_s *um;
FAR const struct mountpt_operations *ops;
int ret = -EPERM;
finfo("filep=%p buf=%p\n", filep, buf);
@ -1335,14 +1268,6 @@ static int unionfs_fstat(FAR const struct file *filep, FAR struct stat *buf)
DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
ui = (FAR struct unionfs_inode_s *)filep->f_inode->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(ui != NULL && filep->f_priv != NULL);
uf = (FAR struct unionfs_file_s *)filep->f_priv;
@ -1355,13 +1280,7 @@ static int unionfs_fstat(FAR const struct file *filep, FAR struct stat *buf)
/* Perform the lower level write operation */
if (ops->fstat != NULL)
{
ret = ops->fstat(&uf->uf_file, buf);
}
unionfs_semgive(ui);
return ret;
return ops->fstat ? ops->fstat(&uf->uf_file, buf) : -EPERM;
}
/****************************************************************************
@ -1378,7 +1297,6 @@ static int unionfs_truncate(FAR struct file *filep, off_t length)
FAR struct unionfs_file_s *uf;
FAR struct unionfs_mountpt_s *um;
FAR const struct mountpt_operations *ops;
int ret = -EPERM;
finfo("filep=%p length=%ld\n", filep, (long)length);
@ -1387,14 +1305,6 @@ static int unionfs_truncate(FAR struct file *filep, off_t length)
DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
ui = (FAR struct unionfs_inode_s *)filep->f_inode->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(ui != NULL && filep->f_priv != NULL);
uf = (FAR struct unionfs_file_s *)filep->f_priv;
@ -1407,13 +1317,7 @@ static int unionfs_truncate(FAR struct file *filep, off_t length)
/* Perform the lower level write operation */
if (ops->truncate != NULL)
{
ret = ops->truncate(&uf->uf_file, length);
}
unionfs_semgive(ui);
return ret;
return ops->truncate ? ops->truncate(&uf->uf_file, length) : -EPERM;
}
/****************************************************************************
@ -1940,7 +1844,7 @@ static int unionfs_rewinddir(struct inode *mountpt, struct fs_dirent_s *dir)
FAR struct unionfs_mountpt_s *um;
FAR const struct mountpt_operations *ops;
FAR struct fs_unionfsdir_s *fu;
int ret;
int ret = -EINVAL;
finfo("mountpt=%p dir=%p\n", mountpt, dir);
@ -1949,14 +1853,6 @@ static int unionfs_rewinddir(struct inode *mountpt, struct fs_dirent_s *dir)
DEBUGASSERT(mountpt != NULL && mountpt->i_private != NULL);
ui = (FAR struct unionfs_inode_s *)mountpt->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(dir);
fu = &dir->u.unionfs;
@ -1988,13 +1884,8 @@ static int unionfs_rewinddir(struct inode *mountpt, struct fs_dirent_s *dir)
ret = ops->rewinddir(um->um_node, fu->fu_lower[fu->fu_ndx]);
dir->fd_position = fu->fu_lower[fu->fu_ndx]->fd_position;
}
else
{
ret = -EINVAL;
}
}
unionfs_semgive(ui);
return ret;
}
@ -2085,10 +1976,14 @@ static int unionfs_unbind(FAR void *handle, FAR struct inode **blkdriver,
if (ui->ui_nopen <= 0)
{
unionfs_semgive(ui);
unionfs_destroy(ui);
}
else
{
unionfs_semgive(ui);
}
unionfs_semgive(ui);
return OK;
}
@ -2119,14 +2014,6 @@ static int unionfs_statfs(FAR struct inode *mountpt, FAR struct statfs *buf)
memset(buf, 0, sizeof(struct statfs));
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
/* Get statfs info from file system 1.
*
* REVISIT: What would it mean if one file system did not support statfs?
@ -2149,7 +2036,7 @@ static int unionfs_statfs(FAR struct inode *mountpt, FAR struct statfs *buf)
ret = ops1->statfs(um1->um_node, &buf1);
if (ret < 0)
{
goto errout_with_semaphore;
return ret;
}
/* Get stafs info from file system 2 */
@ -2157,29 +2044,26 @@ static int unionfs_statfs(FAR struct inode *mountpt, FAR struct statfs *buf)
ret = ops2->statfs(um2->um_node, &buf2);
if (ret < 0)
{
goto errout_with_semaphore;
return ret;
}
}
else if (ops1->statfs != NULL)
{
/* We have statfs for file system 1 only */
ret = ops1->statfs(um1->um_node, buf);
goto errout_with_semaphore;
return ops1->statfs(um1->um_node, buf);
}
else if (ops2->statfs != NULL)
{
/* We have statfs for file system 2 only */
ret = ops2->statfs(um2->um_node, buf);
goto errout_with_semaphore;
return ops2->statfs(um2->um_node, buf);
}
else
{
/* We could not get stafs info from either file system */
ret = -ENOSYS;
goto errout_with_semaphore;
return -ENOSYS;
}
/* We get here is we successfully obtained statfs info from both file
@ -2231,11 +2115,7 @@ static int unionfs_statfs(FAR struct inode *mountpt, FAR struct statfs *buf)
buf->f_bfree = buf1.f_bfree + buf2.f_bfree;
buf->f_bavail = buf1.f_bavail + buf2.f_bavail;
ret = OK;
errout_with_semaphore:
unionfs_semgive(ui);
return ret;
return OK;
}
/****************************************************************************
@ -2258,14 +2138,6 @@ static int unionfs_unlink(FAR struct inode *mountpt,
relpath != NULL);
ui = (FAR struct unionfs_inode_s *)mountpt->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
/* Check if some exists at this path on file system 1. This might be
* a file or a directory
*/
@ -2305,7 +2177,6 @@ static int unionfs_unlink(FAR struct inode *mountpt,
}
}
unionfs_semgive(ui);
return ret;
}
@ -2331,30 +2202,20 @@ static int unionfs_mkdir(FAR struct inode *mountpt, FAR const char *relpath,
relpath != NULL);
ui = (FAR struct unionfs_inode_s *)mountpt->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
/* Is there anything with this name on either file system? */
um = &ui->ui_fs[0];
ret = unionfs_trystat(um->um_node, relpath, um->um_prefix, &buf);
if (ret >= 0)
{
ret = -EEXIST;
goto errout_with_semaphore;
return -EEXIST;
}
um = &ui->ui_fs[1];
ret = unionfs_trystat(um->um_node, relpath, um->um_prefix, &buf);
if (ret >= 0)
{
ret = -EEXIST;
goto errout_with_semaphore;
return -EEXIST;
}
/* Try to create the directory on both file systems. */
@ -2370,20 +2231,7 @@ static int unionfs_mkdir(FAR struct inode *mountpt, FAR const char *relpath,
* read-only and the other is write-able?
*/
if (ret1 >= 0 || ret2 >= 0)
{
ret = OK;
}
else
{
/* Otherwise, pick one */
ret = ret1;
}
errout_with_semaphore:
unionfs_semgive(ui);
return ret;
return (ret1 >= 0 || ret2 >= 0) ? OK : ret1;
}
/****************************************************************************
@ -2394,8 +2242,8 @@ static int unionfs_rmdir(FAR struct inode *mountpt, FAR const char *relpath)
{
FAR struct unionfs_inode_s *ui;
FAR struct unionfs_mountpt_s *um;
int ret = -ENOENT;
int tmp;
int ret;
finfo("relpath: %s\n", relpath);
@ -2405,16 +2253,6 @@ static int unionfs_rmdir(FAR struct inode *mountpt, FAR const char *relpath)
relpath != NULL);
ui = (FAR struct unionfs_inode_s *)mountpt->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
ret = -ENOENT;
/* We really don't know any better so we will try to remove the directory
* from both file systems.
*/
@ -2432,7 +2270,6 @@ static int unionfs_rmdir(FAR struct inode *mountpt, FAR const char *relpath)
ret = unionfs_tryrmdir(um->um_node, relpath, um->um_prefix);
if (ret < 0)
{
unionfs_semgive(ui);
return ret;
}
}
@ -2456,7 +2293,6 @@ static int unionfs_rmdir(FAR struct inode *mountpt, FAR const char *relpath)
*/
}
unionfs_semgive(ui);
return ret;
}
@ -2470,8 +2306,8 @@ static int unionfs_rename(FAR struct inode *mountpt,
{
FAR struct unionfs_inode_s *ui;
FAR struct unionfs_mountpt_s *um;
int tmp;
int ret = -ENOENT;
int tmp;
finfo("oldrelpath: %s newrelpath: %s\n", oldrelpath, newrelpath);
@ -2480,14 +2316,6 @@ static int unionfs_rename(FAR struct inode *mountpt,
DEBUGASSERT(mountpt != NULL && mountpt->i_private != NULL);
ui = (FAR struct unionfs_inode_s *)mountpt->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
DEBUGASSERT(oldrelpath != NULL && oldrelpath != NULL);
/* Is there a file with this name on file system 1 */
@ -2510,7 +2338,6 @@ static int unionfs_rename(FAR struct inode *mountpt,
* file of the same relative path will become visible.
*/
unionfs_semgive(ui);
return OK;
}
}
@ -2532,7 +2359,6 @@ static int unionfs_rename(FAR struct inode *mountpt,
um->um_prefix);
}
unionfs_semgive(ui);
return ret;
}
@ -2555,14 +2381,6 @@ static int unionfs_stat(FAR struct inode *mountpt, FAR const char *relpath,
relpath != NULL);
ui = (FAR struct unionfs_inode_s *)mountpt->i_private;
/* Get exclusive access to the file system data structures */
ret = unionfs_semtake(ui, false);
if (ret < 0)
{
return ret;
}
/* stat this path on file system 1 */
um = &ui->ui_fs[0];
@ -2573,7 +2391,6 @@ static int unionfs_stat(FAR struct inode *mountpt, FAR const char *relpath,
* shadow the second anyway.
*/
unionfs_semgive(ui);
return OK;
}
@ -2587,7 +2404,6 @@ static int unionfs_stat(FAR struct inode *mountpt, FAR const char *relpath,
* shadow the second anyway.
*/
unionfs_semgive(ui);
return OK;
}
@ -2620,7 +2436,6 @@ static int unionfs_stat(FAR struct inode *mountpt, FAR const char *relpath,
}
}
unionfs_semgive(ui);
return ret;
}
@ -2818,17 +2633,6 @@ int unionfs_mount(FAR const char *fspath1, FAR const char *prefix1,
* for now, however.
*/
/* Insert a dummy node -- we need to hold the inode semaphore
* to do this because we will have a momentarily bad structure.
* NOTE that the inode will be created with a reference count of zero.
*/
ret = inode_semtake();
if (ret < 0)
{
return ret;
}
ret = inode_reserve(mountpt, &mpinode);
if (ret < 0)
{
@ -2841,7 +2645,7 @@ int unionfs_mount(FAR const char *fspath1, FAR const char *prefix1,
*/
ferr("ERROR: Failed to reserve inode\n");
goto errout_with_semaphore;
return ret;
}
/* Populate the inode with driver specific information. */
@ -2862,14 +2666,10 @@ int unionfs_mount(FAR const char *fspath1, FAR const char *prefix1,
goto errout_with_mountpt;
}
inode_semgive();
return OK;
errout_with_mountpt:
inode_release(mpinode);
errout_with_semaphore:
inode_semgive();
return ret;
}
#endif /* !CONFIG_DISABLE_MOUNTPOINT && CONFIG_FS_UNIONFS */