From 2b71007c4f840afe83d0c98ec4f552e852e38844 Mon Sep 17 00:00:00 2001 From: chao an Date: Wed, 31 Aug 2022 21:26:10 +0800 Subject: [PATCH] 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 ) at utils/net_lock.c:427 | #1 0x000055555557489c in usrsockdev_do_request (conn=0x5555555f8800 , iov=0x7ffff3f36040, iovcnt=1) at usrsock/usrsock_dev.c:1185 | --> send close request to usrsock deamon | | #2 0x00005555555d0439 in do_close_request (conn=0x5555555f8800 ) at usrsock/usrsock_close.c:109 | #3 0x00005555555d04f5 in usrsock_close (conn=0x5555555f8800 ) 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 ) 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 ) 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 , 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 ) at usrsocktest_daemon.c:1846 | #7 0x000055555559161e in pthread_startup (entry=0x5555555c60d3 , arg=0x5555555f5360 ) 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 --- 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 2799c62902e0d..d1a9a22a1479e 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; }