Received: by 2002:a25:ab43:0:0:0:0:0 with SMTP id u61csp6591028ybi; Wed, 29 May 2019 09:58:25 -0700 (PDT) X-Google-Smtp-Source: APXvYqxVep+IwbIJPTu2NeXR5v3TW8k5RMVmaFDRPw/pQykeXi2vWH+onUE+gG4Pv2+II4YYRB5j X-Received: by 2002:a63:c750:: with SMTP id v16mr138704821pgg.409.1559149105225; Wed, 29 May 2019 09:58:25 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1559149105; cv=none; d=google.com; s=arc-20160816; b=r0wOZe9KJAbvKHzWTT/97zVp4W7NhH6SRNuQlGFbGkwKI7c0N0EtVQmRY7wiwla2RM vkxRtO9aar3eMLrUHBAN8rt/nYBKK1uEfnX7qxMDQvuprd2UAZ3u9yamDL4vx12BoUbY D2rDO3d3SWdEiXavDCn+Vb8cE0Hd40mJGPHclGWKu/QxstLBAeu9GzWJUiHwlVVCqfw3 YE1pcVd4aXu/l5yW9LwtZDiQOXDmXAioFc1AxSLik3wBRZ7xobU0JvmB2VXqdqt4urwn fvoIW0pjO4+EiziBLib3TVSLriQ6wSACYv/eAKc0phRhYsoWFqe4nSdNytUbsH3peKKn HGnQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=w4GoFFZmIO6ONfH5CRW6Gu/SdUNbSdET1Dq1hy75qoE=; b=DCXz5SSsQ+sbOtVkdexjIGvrDVxade3/yU+9U4oLOsvBR3bmyM9dXf4ANm/49yLSEn y4QDZZMd4WohMcXszDbilSKwinO6IsV/823HyW9jhokYcs4mZBLJFFjfdnAw59UHyu5V 6U9Ynexa7KJj5Lyq7yoYGXJQBXHR4du9TUY8J5Hg938kixLEyO2Z4ZgGV0ldZNA68Xsy QTeOlWWGeqVv/1LpzwcE0qFOrj+LZMXl2iZ6UJUHddoQ2HGs4pVARkNs0/JGOy1/6T4W /qZ0EfftNx1toBcRIU2WOGcd+la43VcmFq/ol4XGbTIFZkV2SpoClOpdLVodbYg5/SsO FlOA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=czcStmmc; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n1si195549pgl.216.2019.05.29.09.58.07; Wed, 29 May 2019 09:58:25 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=czcStmmc; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726820AbfE2Q44 (ORCPT + 99 others); Wed, 29 May 2019 12:56:56 -0400 Received: from mail-it1-f193.google.com ([209.85.166.193]:37046 "EHLO mail-it1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726125AbfE2Q4z (ORCPT ); Wed, 29 May 2019 12:56:55 -0400 Received: by mail-it1-f193.google.com with SMTP id s16so4718458ita.2; Wed, 29 May 2019 09:56:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=w4GoFFZmIO6ONfH5CRW6Gu/SdUNbSdET1Dq1hy75qoE=; b=czcStmmc2/6LbnsPYs51brMrGPhnOnADuiA6IeAW9A04U8ZISckf843V37BQUBor/h LNjzRKjkK+I5ZQi0Y5u4XqoYGZnr+Nto2hnZKDfGg49Sn9M5vPHN8jQaH9ImVBeFwWSa aCaubmHSW0QtJrFhySwVVCq3IPbFnFsmy6mf+hAPTfeNyeSGk15hqgwxgnH8C17wEv/G SbgjlEeXac2TsIT+lA7+n9l4zVQTHD+aIyEqHV93nXce/ENh3Ooxty6NgeUI4S0BV+qY DN+d2Kg32RZcQYDNueaoNlIF9zk/l0e2cFMpN3sEGwmd5fMFrCGRBoQqxSDSG6dRtE4U suiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=w4GoFFZmIO6ONfH5CRW6Gu/SdUNbSdET1Dq1hy75qoE=; b=tAQa8AXXD5KIEBgOgtGffU+bkqy2xkbyIhkEyS4HsRU3kM3ki6hDoZmetRGbUe7rFV 05WHQOh4OgA0RGOaQsm59l2fIbXHgTWuE7O2RqWXqheRrhJGJzCI+FguxnLkh6QBQRnt icx8JCsMhIAwxxYECLSveHe+Uvo1j1Pu7OHjTx6PXhMldWrEvS0oLCiWvxkEvJdhLDH9 6gVg81koDCrcSXSDuLSiMEgiRqMyUgEO9zsdfOMVryCcX79pAJQIZQj4h+PEMOc90OvS B0ABySSf0NqUwaZfUVTa9Nvx1k/loeqdcPwWGlNKM9qNf2Z6ucNyEKTd9oqEvQ47fSuz 8Hhw== X-Gm-Message-State: APjAAAWO/yeZKpxfjjwndPc/3wxY1qXHi1R9HaJxcJi+K91CotHoVaWu Mg0JlBx1rxXZ4YUOi4w+AtgRJC5330JiMEEgbdA= X-Received: by 2002:a05:6638:621:: with SMTP id h1mr8424841jar.5.1559149014013; Wed, 29 May 2019 09:56:54 -0700 (PDT) MIME-Version: 1.0 References: <20190522032144.10995-1-deepa.kernel@gmail.com> <20190529161157.GA27659@redhat.com> In-Reply-To: <20190529161157.GA27659@redhat.com> From: Deepa Dinamani Date: Wed, 29 May 2019 09:56:43 -0700 Message-ID: Subject: Re: pselect/etc semantics (Was: [PATCH v2] signal: Adjust error codes according to restore_user_sigmask()) To: Oleg Nesterov Cc: Al Viro , "Eric W. Biederman" , Linus Torvalds , Linux Kernel Mailing List , Andrew Morton , Arnd Bergmann , dbueso@suse.de, axboe@kernel.dk, Davidlohr Bueso , Eric Wong , Jason Baron , Linux FS-devel Mailing List , linux-aio , Omar Kilani , Thomas Gleixner , stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Resending due to inadvertent conversion of prior message to html. On Wed, May 29, 2019 at 9:12 AM Oleg Nesterov wrote: > Al, Linus, Eric, please help. > > The previous discussion was very confusing, we simply can not understand each > other. > > To me everything looks very simple and clear, but perhaps I missed something > obvious? Please correct me. > > I think that the following code is correct > > int interrupted = 0; > > void sigint_handler(int sig) > { > interrupted = 1; > } > > int main(void) > { > sigset_t sigint, empty; > > sigemptyset(&sigint); > sigaddset(&sigint, SIGINT); > sigprocmask(SIG_BLOCK, &sigint, NULL); > > signal(SIGINT, sigint_handler); > > sigemptyset(&empty); // so pselect() unblocks SIGINT > > ret = pselect(..., &empty); > > if (ret >= 0) // sucess or timeout > assert(!interrupted); > > if (interrupted) > assert(ret == -EINTR); > } > > IOW, if pselect(sigmask) temporary unblocks SIGINT according to sigmask, this > signal should not be delivered if a ready fd was found or timeout. The signal > handle should only run if ret == -EINTR. > > (pselect() can be interrupted by any other signal which has a handler. In this > case the handler can be called even if ret >= 0. This is correct, I fail to > understand why some people think this is wrong, and in any case we simply can't > avoid this). > > This was true until 854a6ed56839a ("signal: Add restore_user_sigmask()"), > now this is broken by the signal_pending() check in restore_user_sigmask(). > > This patch https://lore.kernel.org/lkml/20190522032144.10995-1-deepa.kernel@gmail.com/ > turns 0 into -EINTR if signal_pending(), but I think we should simply restore > the old behaviour and simplify the code. > > See the compile-tested patch at the end. Of course, the new _xxx() helpers > should be renamed somehow. fs/aio.c doesn't look right with or without this > patch, but iiuc this is what it did before 854a6ed56839a. > > Let me show the code with the patch applied. I am using epoll_pwait() as an > example because it looks very simple. > > > static inline void set_restore_sigmask(void) > { > // WARN_ON(!TIF_SIGPENDING) was removed by this patch > current->restore_sigmask = true; > } > > int set_xxx(const sigset_t __user *umask, size_t sigsetsize) > { > sigset_t *kmask; > > if (!umask) > return 0; > if (sigsetsize != sizeof(sigset_t)) > return -EINVAL; > if (copy_from_user(kmask, umask, sizeof(sigset_t))) > return -EFAULT; > > // we can safely modify ->saved_sigmask/restore_sigmask, they has no meaning > // until the syscall returns. > set_restore_sigmask(); > current->saved_sigmask = current->blocked; > set_current_blocked(kmask); > > return 0; > } > > > void update_xxx(bool interrupted) > { > // the main reason for this helper is WARN_ON(!TIF_SIGPENDING) which was "moved" > // from set_restore_sigmask() above. > if (interrupted) > WARN_ON(!test_thread_flag(TIF_SIGPENDING)); > else > restore_saved_sigmask(); > } > > SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events, > int, maxevents, int, timeout, const sigset_t __user *, sigmask, > size_t, sigsetsize) > { > int error; > > error = set_xxx(sigmask, sigsetsize); > if (error) > return error; > > error = do_epoll_wait(epfd, events, maxevents, timeout); > update_xxx(error == -EINTR); > > return error; > } > > Oleg. > --- > > fs/aio.c | 40 ++++++++++++++--------- > fs/eventpoll.c | 12 +++---- > fs/io_uring.c | 12 +++---- > fs/select.c | 40 +++++++---------------- > include/linux/compat.h | 4 +-- > include/linux/sched/signal.h | 2 -- > include/linux/signal.h | 6 ++-- > kernel/signal.c | 77 ++++++++++++++++---------------------------- > 8 files changed, 74 insertions(+), 119 deletions(-) > > > diff --git a/fs/aio.c b/fs/aio.c > index 3490d1f..8315bd2 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -2093,8 +2093,8 @@ SYSCALL_DEFINE6(io_pgetevents, > const struct __aio_sigset __user *, usig) > { > struct __aio_sigset ksig = { NULL, }; > - sigset_t ksigmask, sigsaved; > struct timespec64 ts; > + bool interrupted; > int ret; > > if (timeout && unlikely(get_timespec64(&ts, timeout))) > @@ -2103,13 +2103,15 @@ SYSCALL_DEFINE6(io_pgetevents, I already admitted I was wrong about this. And the behavior you point out is consistent with man page. I'm not sure why we are not understanding that I'm agreeing with you: https://lore.kernel.org/lkml/CABeXuvouBzZuNarmNcd9JgZgvonL1N_p21gat=O_x0-1hMx=6A@mail.gmail.com/ This was true until 854a6ed56839a("signal: Add restore_user_sigmask()"), now this is broken by the signal_pending() check in restore_user_sigmask(). This patch is wrong because I did not know about the above behavior before. I also admitted to this. And proposed another way to revert the patch. This patch https://lore.kernel.org/lkml/20190522032144.10995-1-deepa.kernel@gmail.com/ turns 0 into -EINTR if signal_pending(), but I think we should simply restore the old behaviour and simplify the code. I agree that the patch that turns 0 to -EINTR should not be applied. It was my misunderstanding. The part I'm not sure is that I cannot find anything that is wrong with 854a6ed56839a("signal: Add restore_user_sigmask()") that was not already before. But, that is ok, we can do the revert and I will investigate with the test application if I can find something. -Deepa > if (usig && copy_from_user(&ksig, usig, sizeof(ksig))) > return -EFAULT; > > - ret = set_user_sigmask(ksig.sigmask, &ksigmask, &sigsaved, ksig.sigsetsize); > + ret = set_xxx(ksig.sigmask, ksig.sigsetsize); > if (ret) > return ret; > > ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL); > - restore_user_sigmask(ksig.sigmask, &sigsaved); > - if (signal_pending(current) && !ret) > + > + interrupted = signal_pending(current); > + update_xxx(interrupted); > + if (interrupted && !ret) > ret = -ERESTARTNOHAND; > > return ret; > @@ -2126,8 +2128,8 @@ SYSCALL_DEFINE6(io_pgetevents_time32, > const struct __aio_sigset __user *, usig) > { > struct __aio_sigset ksig = { NULL, }; > - sigset_t ksigmask, sigsaved; > struct timespec64 ts; > + bool interrupted; > int ret; > > if (timeout && unlikely(get_old_timespec32(&ts, timeout))) > @@ -2137,13 +2139,15 @@ SYSCALL_DEFINE6(io_pgetevents_time32, > return -EFAULT; > > > - ret = set_user_sigmask(ksig.sigmask, &ksigmask, &sigsaved, ksig.sigsetsize); > + ret = set_xxx(ksig.sigmask, ksig.sigsetsize); > if (ret) > return ret; > > ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL); > - restore_user_sigmask(ksig.sigmask, &sigsaved); > - if (signal_pending(current) && !ret) > + > + interrupted = signal_pending(current); > + update_xxx(interrupted); > + if (interrupted && !ret) > ret = -ERESTARTNOHAND; > > return ret; > @@ -2191,8 +2195,8 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents, > const struct __compat_aio_sigset __user *, usig) > { > struct __compat_aio_sigset ksig = { NULL, }; > - sigset_t ksigmask, sigsaved; > struct timespec64 t; > + bool interrupted; > int ret; > > if (timeout && get_old_timespec32(&t, timeout)) > @@ -2201,13 +2205,15 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents, > if (usig && copy_from_user(&ksig, usig, sizeof(ksig))) > return -EFAULT; > > - ret = set_compat_user_sigmask(ksig.sigmask, &ksigmask, &sigsaved, ksig.sigsetsize); > + ret = set_compat_xxx(ksig.sigmask, ksig.sigsetsize); > if (ret) > return ret; > > ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL); > - restore_user_sigmask(ksig.sigmask, &sigsaved); > - if (signal_pending(current) && !ret) > + > + interrupted = signal_pending(current); > + update_xxx(interrupted); > + if (interrupted && !ret) > ret = -ERESTARTNOHAND; > > return ret; > @@ -2224,8 +2230,8 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents_time64, > const struct __compat_aio_sigset __user *, usig) > { > struct __compat_aio_sigset ksig = { NULL, }; > - sigset_t ksigmask, sigsaved; > struct timespec64 t; > + bool interrupted; > int ret; > > if (timeout && get_timespec64(&t, timeout)) > @@ -2234,13 +2240,15 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents_time64, > if (usig && copy_from_user(&ksig, usig, sizeof(ksig))) > return -EFAULT; > > - ret = set_compat_user_sigmask(ksig.sigmask, &ksigmask, &sigsaved, ksig.sigsetsize); > + ret = set_compat_xxx(ksig.sigmask, ksig.sigsetsize); > if (ret) > return ret; > > ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL); > - restore_user_sigmask(ksig.sigmask, &sigsaved); > - if (signal_pending(current) && !ret) > + > + interrupted = signal_pending(current); > + update_xxx(interrupted); > + if (interrupted && !ret) > ret = -ERESTARTNOHAND; > > return ret; > diff --git a/fs/eventpoll.c b/fs/eventpoll.c > index 4a0e98d..0b1a337 100644 > --- a/fs/eventpoll.c > +++ b/fs/eventpoll.c > @@ -2318,19 +2318,17 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events, > size_t, sigsetsize) > { > int error; > - sigset_t ksigmask, sigsaved; > > /* > * If the caller wants a certain signal mask to be set during the wait, > * we apply it here. > */ > - error = set_user_sigmask(sigmask, &ksigmask, &sigsaved, sigsetsize); > + error = set_xxx(sigmask, sigsetsize); > if (error) > return error; > > error = do_epoll_wait(epfd, events, maxevents, timeout); > - > - restore_user_sigmask(sigmask, &sigsaved); > + update_xxx(error == -EINTR); > > return error; > } > @@ -2343,19 +2341,17 @@ COMPAT_SYSCALL_DEFINE6(epoll_pwait, int, epfd, > compat_size_t, sigsetsize) > { > long err; > - sigset_t ksigmask, sigsaved; > > /* > * If the caller wants a certain signal mask to be set during the wait, > * we apply it here. > */ > - err = set_compat_user_sigmask(sigmask, &ksigmask, &sigsaved, sigsetsize); > + err = set_compat_xxx(sigmask, sigsetsize); > if (err) > return err; > > err = do_epoll_wait(epfd, events, maxevents, timeout); > - > - restore_user_sigmask(sigmask, &sigsaved); > + update_xxx(err == -EINTR); > > return err; > } > diff --git a/fs/io_uring.c b/fs/io_uring.c > index 310f8d1..b5b99e2 100644 > --- a/fs/io_uring.c > +++ b/fs/io_uring.c > @@ -2180,7 +2180,6 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events, > const sigset_t __user *sig, size_t sigsz) > { > struct io_cq_ring *ring = ctx->cq_ring; > - sigset_t ksigmask, sigsaved; > int ret; > > if (io_cqring_events(ring) >= min_events) > @@ -2189,24 +2188,21 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events, > if (sig) { > #ifdef CONFIG_COMPAT > if (in_compat_syscall()) > - ret = set_compat_user_sigmask((const compat_sigset_t __user *)sig, > - &ksigmask, &sigsaved, sigsz); > + ret = set_compat_xxx((const compat_sigset_t __user *)sig, > + sigsz); > else > #endif > - ret = set_user_sigmask(sig, &ksigmask, > - &sigsaved, sigsz); > + ret = set_xxx(sig, sigsz); > > if (ret) > return ret; > } > > ret = wait_event_interruptible(ctx->wait, io_cqring_events(ring) >= min_events); > + update_xxx(ret == -ERESTARTSYS); > if (ret == -ERESTARTSYS) > ret = -EINTR; > > - if (sig) > - restore_user_sigmask(sig, &sigsaved); > - > return READ_ONCE(ring->r.head) == READ_ONCE(ring->r.tail) ? ret : 0; > } > > diff --git a/fs/select.c b/fs/select.c > index 6cbc9ff..7eab132 100644 > --- a/fs/select.c > +++ b/fs/select.c > @@ -730,7 +730,6 @@ static long do_pselect(int n, fd_set __user *inp, fd_set __user *outp, > const sigset_t __user *sigmask, size_t sigsetsize, > enum poll_time_type type) > { > - sigset_t ksigmask, sigsaved; > struct timespec64 ts, end_time, *to = NULL; > int ret; > > @@ -753,15 +752,14 @@ static long do_pselect(int n, fd_set __user *inp, fd_set __user *outp, > return -EINVAL; > } > > - ret = set_user_sigmask(sigmask, &ksigmask, &sigsaved, sigsetsize); > + ret = set_xxx(sigmask, sigsetsize); > if (ret) > return ret; > > ret = core_sys_select(n, inp, outp, exp, to); > + update_xxx(ret == -ERESTARTNOHAND); > ret = poll_select_copy_remaining(&end_time, tsp, type, ret); > > - restore_user_sigmask(sigmask, &sigsaved); > - > return ret; > } > > @@ -1087,7 +1085,6 @@ SYSCALL_DEFINE5(ppoll, struct pollfd __user *, ufds, unsigned int, nfds, > struct __kernel_timespec __user *, tsp, const sigset_t __user *, sigmask, > size_t, sigsetsize) > { > - sigset_t ksigmask, sigsaved; > struct timespec64 ts, end_time, *to = NULL; > int ret; > > @@ -1100,18 +1097,16 @@ SYSCALL_DEFINE5(ppoll, struct pollfd __user *, ufds, unsigned int, nfds, > return -EINVAL; > } > > - ret = set_user_sigmask(sigmask, &ksigmask, &sigsaved, sigsetsize); > + ret = set_xxx(sigmask, sigsetsize); > if (ret) > return ret; > > ret = do_sys_poll(ufds, nfds, to); > > - restore_user_sigmask(sigmask, &sigsaved); > - > + update_xxx(ret == -EINTR); > /* We can restart this syscall, usually */ > if (ret == -EINTR) > ret = -ERESTARTNOHAND; > - > ret = poll_select_copy_remaining(&end_time, tsp, PT_TIMESPEC, ret); > > return ret; > @@ -1123,7 +1118,6 @@ SYSCALL_DEFINE5(ppoll_time32, struct pollfd __user *, ufds, unsigned int, nfds, > struct old_timespec32 __user *, tsp, const sigset_t __user *, sigmask, > size_t, sigsetsize) > { > - sigset_t ksigmask, sigsaved; > struct timespec64 ts, end_time, *to = NULL; > int ret; > > @@ -1136,18 +1130,16 @@ SYSCALL_DEFINE5(ppoll_time32, struct pollfd __user *, ufds, unsigned int, nfds, > return -EINVAL; > } > > - ret = set_user_sigmask(sigmask, &ksigmask, &sigsaved, sigsetsize); > + ret = set_xxx(sigmask, sigsetsize); > if (ret) > return ret; > > ret = do_sys_poll(ufds, nfds, to); > > - restore_user_sigmask(sigmask, &sigsaved); > - > + update_xxx(ret == -EINTR); > /* We can restart this syscall, usually */ > if (ret == -EINTR) > ret = -ERESTARTNOHAND; > - > ret = poll_select_copy_remaining(&end_time, tsp, PT_OLD_TIMESPEC, ret); > > return ret; > @@ -1322,7 +1314,6 @@ static long do_compat_pselect(int n, compat_ulong_t __user *inp, > void __user *tsp, compat_sigset_t __user *sigmask, > compat_size_t sigsetsize, enum poll_time_type type) > { > - sigset_t ksigmask, sigsaved; > struct timespec64 ts, end_time, *to = NULL; > int ret; > > @@ -1345,15 +1336,14 @@ static long do_compat_pselect(int n, compat_ulong_t __user *inp, > return -EINVAL; > } > > - ret = set_compat_user_sigmask(sigmask, &ksigmask, &sigsaved, sigsetsize); > + ret = set_compat_xxx(sigmask, sigsetsize); > if (ret) > return ret; > > ret = compat_core_sys_select(n, inp, outp, exp, to); > + update_xxx(ret == -ERESTARTNOHAND); > ret = poll_select_copy_remaining(&end_time, tsp, type, ret); > > - restore_user_sigmask(sigmask, &sigsaved); > - > return ret; > } > > @@ -1406,7 +1396,6 @@ COMPAT_SYSCALL_DEFINE5(ppoll_time32, struct pollfd __user *, ufds, > unsigned int, nfds, struct old_timespec32 __user *, tsp, > const compat_sigset_t __user *, sigmask, compat_size_t, sigsetsize) > { > - sigset_t ksigmask, sigsaved; > struct timespec64 ts, end_time, *to = NULL; > int ret; > > @@ -1419,18 +1408,16 @@ COMPAT_SYSCALL_DEFINE5(ppoll_time32, struct pollfd __user *, ufds, > return -EINVAL; > } > > - ret = set_compat_user_sigmask(sigmask, &ksigmask, &sigsaved, sigsetsize); > + ret = set_compat_xxx(sigmask, sigsetsize); > if (ret) > return ret; > > ret = do_sys_poll(ufds, nfds, to); > > - restore_user_sigmask(sigmask, &sigsaved); > - > + update_xxx(ret == -EINTR); > /* We can restart this syscall, usually */ > if (ret == -EINTR) > ret = -ERESTARTNOHAND; > - > ret = poll_select_copy_remaining(&end_time, tsp, PT_OLD_TIMESPEC, ret); > > return ret; > @@ -1442,7 +1429,6 @@ COMPAT_SYSCALL_DEFINE5(ppoll_time64, struct pollfd __user *, ufds, > unsigned int, nfds, struct __kernel_timespec __user *, tsp, > const compat_sigset_t __user *, sigmask, compat_size_t, sigsetsize) > { > - sigset_t ksigmask, sigsaved; > struct timespec64 ts, end_time, *to = NULL; > int ret; > > @@ -1455,18 +1441,16 @@ COMPAT_SYSCALL_DEFINE5(ppoll_time64, struct pollfd __user *, ufds, > return -EINVAL; > } > > - ret = set_compat_user_sigmask(sigmask, &ksigmask, &sigsaved, sigsetsize); > + ret = set_compat_xxx(sigmask, sigsetsize); > if (ret) > return ret; > > ret = do_sys_poll(ufds, nfds, to); > > - restore_user_sigmask(sigmask, &sigsaved); > - > + update_xxx(ret == -EINTR); > /* We can restart this syscall, usually */ > if (ret == -EINTR) > ret = -ERESTARTNOHAND; > - > ret = poll_select_copy_remaining(&end_time, tsp, PT_TIMESPEC, ret); > > return ret; > diff --git a/include/linux/compat.h b/include/linux/compat.h > index ebddcb6..b20b001 100644 > --- a/include/linux/compat.h > +++ b/include/linux/compat.h > @@ -138,9 +138,7 @@ typedef struct { > compat_sigset_word sig[_COMPAT_NSIG_WORDS]; > } compat_sigset_t; > > -int set_compat_user_sigmask(const compat_sigset_t __user *usigmask, > - sigset_t *set, sigset_t *oldset, > - size_t sigsetsize); > +int set_compat_xxx(const compat_sigset_t __user *umask, size_t sigsetsize); > > struct compat_sigaction { > #ifndef __ARCH_HAS_IRIX_SIGACTION > diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h > index 38a0f07..8b5b537 100644 > --- a/include/linux/sched/signal.h > +++ b/include/linux/sched/signal.h > @@ -417,7 +417,6 @@ void task_join_group_stop(struct task_struct *task); > static inline void set_restore_sigmask(void) > { > set_thread_flag(TIF_RESTORE_SIGMASK); > - WARN_ON(!test_thread_flag(TIF_SIGPENDING)); > } > > static inline void clear_tsk_restore_sigmask(struct task_struct *task) > @@ -448,7 +447,6 @@ static inline bool test_and_clear_restore_sigmask(void) > static inline void set_restore_sigmask(void) > { > current->restore_sigmask = true; > - WARN_ON(!test_thread_flag(TIF_SIGPENDING)); > } > static inline void clear_tsk_restore_sigmask(struct task_struct *task) > { > diff --git a/include/linux/signal.h b/include/linux/signal.h > index 9702016..65f84ac 100644 > --- a/include/linux/signal.h > +++ b/include/linux/signal.h > @@ -273,10 +273,8 @@ extern int group_send_sig_info(int sig, struct kernel_siginfo *info, > struct task_struct *p, enum pid_type type); > extern int __group_send_sig_info(int, struct kernel_siginfo *, struct task_struct *); > extern int sigprocmask(int, sigset_t *, sigset_t *); > -extern int set_user_sigmask(const sigset_t __user *usigmask, sigset_t *set, > - sigset_t *oldset, size_t sigsetsize); > -extern void restore_user_sigmask(const void __user *usigmask, > - sigset_t *sigsaved); > +extern int set_xxx(const sigset_t __user *umask, size_t sigsetsize); > +extern void update_xxx(bool interupted); > extern void set_current_blocked(sigset_t *); > extern void __set_current_blocked(const sigset_t *); > extern int show_unhandled_signals; > diff --git a/kernel/signal.c b/kernel/signal.c > index d7b9d14..0a1ec68 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -2861,79 +2861,56 @@ EXPORT_SYMBOL(sigprocmask); > * > * This is useful for syscalls such as ppoll, pselect, io_pgetevents and > * epoll_pwait where a new sigmask is passed from userland for the syscalls. > + * > + * Note that it does set_restore_sigmask() in advance, so it must be always > + * paired with update_xxx() before return from syscall. > */ > -int set_user_sigmask(const sigset_t __user *usigmask, sigset_t *set, > - sigset_t *oldset, size_t sigsetsize) > +int set_xxx(const sigset_t __user *umask, size_t sigsetsize) > { > - if (!usigmask) > - return 0; > + sigset_t *kmask; > > + if (!umask) > + return 0; > if (sigsetsize != sizeof(sigset_t)) > return -EINVAL; > - if (copy_from_user(set, usigmask, sizeof(sigset_t))) > + if (copy_from_user(kmask, umask, sizeof(sigset_t))) > return -EFAULT; > > - *oldset = current->blocked; > - set_current_blocked(set); > + set_restore_sigmask(); > + current->saved_sigmask = current->blocked; > + set_current_blocked(kmask); > > return 0; > } > -EXPORT_SYMBOL(set_user_sigmask); > + > +void update_xxx(bool interrupted) > +{ > + if (interrupted) > + WARN_ON(!test_thread_flag(TIF_SIGPENDING)); > + else > + restore_saved_sigmask(); > +} > > #ifdef CONFIG_COMPAT > -int set_compat_user_sigmask(const compat_sigset_t __user *usigmask, > - sigset_t *set, sigset_t *oldset, > - size_t sigsetsize) > +int set_compat_xxx(const compat_sigset_t __user *umask, size_t sigsetsize) > { > - if (!usigmask) > - return 0; > + sigset_t *kmask; > > + if (!umask) > + return 0; > if (sigsetsize != sizeof(compat_sigset_t)) > return -EINVAL; > - if (get_compat_sigset(set, usigmask)) > + if (get_compat_sigset(kmask, umask)) > return -EFAULT; > > - *oldset = current->blocked; > - set_current_blocked(set); > + set_restore_sigmask(); > + current->saved_sigmask = current->blocked; > + set_current_blocked(kmask); > > return 0; > } > -EXPORT_SYMBOL(set_compat_user_sigmask); > #endif > > -/* > - * restore_user_sigmask: > - * usigmask: sigmask passed in from userland. > - * sigsaved: saved sigmask when the syscall started and changed the sigmask to > - * usigmask. > - * > - * This is useful for syscalls such as ppoll, pselect, io_pgetevents and > - * epoll_pwait where a new sigmask is passed in from userland for the syscalls. > - */ > -void restore_user_sigmask(const void __user *usigmask, sigset_t *sigsaved) > -{ > - > - if (!usigmask) > - return; > - /* > - * When signals are pending, do not restore them here. > - * Restoring sigmask here can lead to delivering signals that the above > - * syscalls are intended to block because of the sigmask passed in. > - */ > - if (signal_pending(current)) { > - current->saved_sigmask = *sigsaved; > - set_restore_sigmask(); > - return; > - } > - > - /* > - * This is needed because the fast syscall return path does not restore > - * saved_sigmask when signals are not pending. > - */ > - set_current_blocked(sigsaved); > -} > -EXPORT_SYMBOL(restore_user_sigmask); > - > /** > * sys_rt_sigprocmask - change the list of currently blocked signals > * @how: whether to add, remove, or set signals >