You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2022/09/01 04:59:51 UTC

[incubator-nuttx] branch master updated: fs/dup2: fix potential deadlock on usrsock

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new 42388f16e9 fs/dup2: fix potential deadlock on usrsock
42388f16e9 is described below

commit 42388f16e90e40ff3fb4e154eb2992879f52a069
Author: chao an <an...@xiaomi.com>
AuthorDate: Wed Aug 31 21:26:10 2022 +0800

    fs/dup2: fix potential deadlock on usrsock
    
    apps/examples/usrsocktest/usrsocktest_basic_daemon.c:
    
    321 static void basic_daemon_dup2(FAR struct usrsocktest_daemon_conf_s *dconf)
    322 {
    ...
    335   ret = dup2(sd2, sd);
    352 }
    
    Usrsocktest Task hold the file group lock and send the close request to usrsock deamon :
    
    | #0  net_lockedwait_uninterruptible (sem=0x5555555f8ba2 <g_usrsockdev+34>) at utils/net_lock.c:427
    | #1  0x000055555557489c in usrsockdev_do_request (conn=0x5555555f8800 <g_usrsock_connections>, iov=0x7ffff3f36040, iovcnt=1) at usrsock/usrsock_dev.c:1185
    |                           --> send close request to usrsock deamon
    |
    | #2  0x00005555555d0439 in do_close_request (conn=0x5555555f8800 <g_usrsock_connections>) at usrsock/usrsock_close.c:109
    | #3  0x00005555555d04f5 in usrsock_close (conn=0x5555555f8800 <g_usrsock_connections>) at usrsock/usrsock_close.c:157
    | #4  0x00005555555cf100 in usrsock_sockif_close (psock=0x7ffff3ea4a60) at usrsock/usrsock_sockif.c:234
    | #5  0x00005555555c7b2f in psock_close (psock=0x7ffff3ea4a60) at socket/net_close.c:102
    | #6  0x000055555557a518 in sock_file_close (filep=0x7ffff3f253d0) at socket/socket.c:115
    | #7  0x000055555557678f in file_close (filep=0x7ffff3f253d0) at vfs/fs_close.c:74
    | #8  0x000055555557694c in file_dup2 (filep1=0x7ffff3f253e8, filep2=0x7ffff3f253d0) at vfs/fs_dup2.c:129
    |                           --->  hold group file list lock  ( _files_semtake(list) )
    |
    | #9  0x0000555555575aab in nx_dup2 (fd1=7, fd2=6) at inode/fs_files.c:451
    | #10 0x0000555555575af3 in dup2 (fd1=7, fd2=6) at inode/fs_files.c:473
    | #11 0x000055555559d937 in basic_daemon_dup2 (dconf=0x5555555f8d80 <usrsocktest_daemon_config>) at usrsocktest_basic_daemon.c:335
    | #12 0x000055555559ed80 in usrsocktest_test_basic_daemon_basic_daemon_dup2 () at usrsocktest_basic_daemon.c:612
    | #13 0x000055555559f18d in usrsocktest_group_basic_daemon_run () at usrsocktest_basic_daemon.c:666
    | #14 0x0000555555599f8d in run_tests (name=0x5555555dc8c3 "basic_daemon", test_fn=0x55555559ef50 <usrsocktest_group_basic_daemon_run>) at usrsocktest_main.c:117
    | #15 0x000055555559a06c in run_all_tests () at usrsocktest_main.c:154
    | #16 0x000055555559a3d1 in usrsocktest_main (argc=1, argv=0x7ffff3f25450) at usrsocktest_main.c:248
    | #17 0x000055555555cad8 in nxtask_startup (entrypt=0x55555559a357 <usrsocktest_main>, argc=1, argv=0x7ffff3f25450) at sched/task_startup.c:70
    | #18 0x0000555555559938 in nxtask_start () at task/task_start.c:134
    
    Usrsock Deamon weakup and setup the poll want to perform close request, but locked on fs_getfilep():
    
    | #0  _files_semtake (list=0x7ffff3f250b8) at inode/fs_files.c:51
    |                           --> Request group lock but which hold by close request, deadlock
    | #1  0x00005555555758b1 in fs_getfilep (fd=5, filep=0x7ffff3f47190) at inode/fs_files.c:375
    | #2  0x00005555555d3064 in poll_fdsetup (fd=5, fds=0x7ffff3f47290, setup=true) at vfs/fs_poll.c:79
    | #3  0x00005555555d3243 in poll_setup (fds=0x7ffff3f47290, nfds=2, sem=0x7ffff3f47206) at vfs/fs_poll.c:139
    | #4  0x00005555555d39a6 in nx_poll (fds=0x7ffff3f47290, nfds=2, timeout=-1) at vfs/fs_poll.c:383
    | #5  0x00005555555d3abd in poll (fds=0x7ffff3f47290, nfds=2, timeout=-1) at vfs/fs_poll.c:501
    |                           --> daemon weak up
    | #6  0x00005555555c62c7 in usrsocktest_daemon (param=0x5555555f5360 <g_ub_daemon>) at usrsocktest_daemon.c:1846
    | #7  0x000055555559161e in pthread_startup (entry=0x5555555c60d3 <usrsocktest_daemon>, arg=0x5555555f5360 <g_ub_daemon>) at pthread/pthread_create.c:59
    | #8  0x00005555555d45f0 in pthread_start () at pthread/pthread_create.c:175
    | #9  0x0000000000000000 in ?? ()
    
    Signed-off-by: chao an <an...@xiaomi.com>
---
 fs/inode/fs_files.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c
index 2799c62902..d1a9a22a14 100644
--- a/fs/inode/fs_files.c
+++ b/fs/inode/fs_files.c
@@ -415,8 +415,15 @@ int fs_getfilep(int fd, FAR struct file **filep)
 int nx_dup2(int fd1, int fd2)
 {
   FAR struct filelist *list;
+  FAR struct file     *filep;
+  FAR struct file      file;
   int ret;
 
+  if (fd1 == fd2)
+    {
+      return fd1;
+    }
+
   /* Get the file descriptor list.  It should not be NULL in this context. */
 
   list = nxsched_get_files();
@@ -446,14 +453,20 @@ int nx_dup2(int fd1, int fd2)
         }
     }
 
+  filep = &list->fl_files[fd2 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]
+                         [fd2 % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK];
+  memcpy(&file, filep, sizeof(struct file));
+  memset(filep, 0,     sizeof(struct file));
+
   /* Perform the dup2 operation */
 
   ret = file_dup2(&list->fl_files[fd1 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]
                                  [fd1 % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK],
-                  &list->fl_files[fd2 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]
-                                 [fd2 % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]);
+                  filep);
   _files_semgive(list);
 
+  file_close(&file);
+
   return ret < 0 ? ret : fd2;
 }