Date: Wed, 17 Jul 2019 22:45:43 +0000 (UTC) From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r350097 - in projects/fuse2: sys/fs/fuse sys/kern sys/sys tests/sys/fs/fusefs Message-ID: <201907172245.x6HMjhxX013913@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Wed Jul 17 22:45:43 2019 New Revision: 350097 URL: https://svnweb.freebsd.org/changeset/base/350097 Log: fusefs: multiple interruptility improvements 1) Don't explicitly not mask SIGKILL. kern_sigprocmask won't allow it to be masked, anyway. 2) Fix an infinite loop bug. If a process received both a maskable signal lower than 9 (like SIGINT) and then received SIGKILL, fticket_wait_answer would spin. msleep would immediately return EINTR, but cursig would return SIGINT, so the sleep would get retried. Fix it by explicitly checking whether SIGKILL has been received. 3) Abandon the sig_isfatal optimization introduced by r346357. That optimization would cause fticket_wait_answer to return immediately, without waiting for a response from the server, if the process were going to exit anyway. However, it's vulnerable to a race: 1) fatal signal is received while fticket_wait_answer is sleeping. 2) fticket_wait_answer sends the FUSE_INTERRUPT operation. 3) fticket_wait_answer determines that the signal was fatal and returns without waiting for a response. 4) Another thread changes the signal to non-fatal. 5) The first thread returns to userspace. Instead of exiting, the process continues. 6) The application receives EINTR, wrongly believes that the operation was successfully interrupted, and restarts it. This could cause problems for non-idempotent operations like FUSE_RENAME. Reported by: kib (the race part) Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_ipc.c projects/fuse2/sys/kern/kern_sig.c projects/fuse2/sys/sys/signalvar.h projects/fuse2/tests/sys/fs/fusefs/interrupt.cc Modified: projects/fuse2/sys/fs/fuse/fuse_ipc.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_ipc.c Wed Jul 17 22:07:43 2019 (r350096) +++ projects/fuse2/sys/fs/fuse/fuse_ipc.c Wed Jul 17 22:45:43 2019 (r350097) @@ -445,7 +445,6 @@ fticket_wait_answer(struct fuse_ticket *ftick) } else { /* May as well block all signals */ SIGFILLSET(blockedset); - SIGDELSET(blockedset, SIGKILL); } stops_deferred = sigdeferstop(SIGDEFERSTOP_SILENT); kern_sigprocmask(td, SIG_BLOCK, NULL, &oldset, 0); @@ -489,8 +488,8 @@ retry: * then it will either respond EINTR to the original operation, * or EAGAIN to the interrupt. */ + sigset_t tmpset; int sig; - bool fatal; SDT_PROBE2(fusefs, , ipc, trace, 4, "fticket_wait_answer: interrupt"); @@ -500,12 +499,13 @@ retry: PROC_LOCK(td->td_proc); mtx_lock(&td->td_proc->p_sigacts->ps_mtx); sig = cursig(td); - fatal = sig_isfatal(td->td_proc, sig); + tmpset = td->td_proc->p_siglist; + SIGSETOR(tmpset, td->td_siglist); mtx_unlock(&td->td_proc->p_sigacts->ps_mtx); PROC_UNLOCK(td->td_proc); fuse_lck_mtx_lock(ftick->tk_aw_mtx); - if (!fatal) { + if (!SIGISMEMBER(tmpset, SIGKILL)) { /* * Block the just-delivered signal while we wait for an * interrupt response Modified: projects/fuse2/sys/kern/kern_sig.c ============================================================================== --- projects/fuse2/sys/kern/kern_sig.c Wed Jul 17 22:07:43 2019 (r350096) +++ projects/fuse2/sys/kern/kern_sig.c Wed Jul 17 22:45:43 2019 (r350097) @@ -929,23 +929,6 @@ osigreturn(struct thread *td, struct osigreturn_args * #endif #endif /* COMPAT_43 */ -/* Would this signal be fatal to the current process, if it were caught ? */ -bool -sig_isfatal(struct proc *p, int sig) -{ - intptr_t act; - int prop; - - mtx_assert(&p->p_sigacts->ps_mtx, MA_OWNED); - act = (intptr_t)p->p_sigacts->ps_sigact[_SIG_IDX(sig)]; - if ((intptr_t)SIG_DFL == act) { - prop = sigprop(sig); - return (0 != (prop & (SIGPROP_KILL | SIGPROP_CORE))); - } else { - return (false); - } -} - /* * Initialize signal state for process 0; * set to ignore signals that are ignored by default. Modified: projects/fuse2/sys/sys/signalvar.h ============================================================================== --- projects/fuse2/sys/sys/signalvar.h Wed Jul 17 22:07:43 2019 (r350096) +++ projects/fuse2/sys/sys/signalvar.h Wed Jul 17 22:45:43 2019 (r350097) @@ -384,7 +384,6 @@ int sigacts_shared(struct sigacts *ps); void sigexit(struct thread *td, int sig) __dead2; int sigev_findtd(struct proc *p, struct sigevent *sigev, struct thread **); int sig_ffs(sigset_t *set); -bool sig_isfatal(struct proc *p, int sig); void siginit(struct proc *p); void signotify(struct thread *td); void sigqueue_delete(struct sigqueue *queue, int sig); Modified: projects/fuse2/tests/sys/fs/fusefs/interrupt.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/interrupt.cc Wed Jul 17 22:07:43 2019 (r350096) +++ projects/fuse2/tests/sys/fs/fusefs/interrupt.cc Wed Jul 17 22:45:43 2019 (r350097) @@ -352,65 +352,6 @@ TEST_F(Interrupt, enosys) } /* - * Upon receipt of a fatal signal, fusefs should return ASAP after sending - * FUSE_INTERRUPT. - */ -TEST_F(Interrupt, fatal_signal) -{ - int status; - pthread_t self; - uint64_t mkdir_unique; - sem_t sem; - - ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno); - self = pthread_self(); - - EXPECT_LOOKUP(FUSE_ROOT_ID, RELDIRPATH0) - .WillOnce(Invoke(ReturnErrno(ENOENT))); - expect_mkdir(&mkdir_unique); - EXPECT_CALL(*m_mock, process( - ResultOf([&](auto in) { - return (in.header.opcode == FUSE_INTERRUPT && - in.body.interrupt.unique == mkdir_unique); - }, Eq(true)), - _) - ).WillOnce(Invoke([&](auto in __unused, auto &out __unused) { - sem_post(&sem); - /* Don't respond. The process should exit anyway */ - })); - - fork(false, &status, [&] { - }, [&]() { - struct sigaction sa; - int r; - pthread_t killer_th; - pthread_t self; - - /* SIGUSR2 terminates the process by default */ - bzero(&sa, sizeof(sa)); - sa.sa_handler = SIG_DFL; - r = sigaction(SIGUSR2, &sa, NULL); - if (r != 0) { - perror("sigaction"); - return 1; - } - self = pthread_self(); - r = pthread_create(&killer_th, NULL, killer, (void*)self); - if (r != 0) { - perror("pthread_create"); - return 1; - } - - mkdir(FULLDIRPATH0, MODE); - return 1; - }); - ASSERT_EQ(SIGUSR2, WTERMSIG(status)); - - EXPECT_EQ(0, sem_wait(&sem)) << strerror(errno); - sem_destroy(&sem); -} - -/* * A FUSE filesystem is legally allowed to ignore INTERRUPT operations, and * complete the original operation whenever it damn well pleases. */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201907172245.x6HMjhxX013913>