From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8786AC28CC0 for ; Wed, 29 May 2019 16:57:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6B12D23E81 for ; Wed, 29 May 2019 16:57:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726498AbfE2Q5Z (ORCPT ); Wed, 29 May 2019 12:57:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39402 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726057AbfE2Q5Y (ORCPT ); Wed, 29 May 2019 12:57:24 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0B2E885538; Wed, 29 May 2019 16:57:23 +0000 (UTC) Received: from dhcp-27-174.brq.redhat.com (unknown [10.43.17.159]) by smtp.corp.redhat.com (Postfix) with SMTP id D39945C1A1; Wed, 29 May 2019 16:57:18 +0000 (UTC) Received: by dhcp-27-174.brq.redhat.com (nbSMTP-1.00) for uid 1000 oleg@redhat.com; Wed, 29 May 2019 18:57:22 +0200 (CEST) Date: Wed, 29 May 2019 18:57:18 +0200 From: Oleg Nesterov To: Deepa Dinamani Cc: David Laight , Linux Kernel Mailing List , Andrew Morton , Alexander Viro , 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" Subject: Re: [PATCH v2] signal: Adjust error codes according to restore_user_sigmask() Message-ID: <20190529165717.GC27659@redhat.com> References: <345cfba5edde470f9a68d913f44fa342@AcuMS.aculab.com> <20190523163604.GE23070@redhat.com> <20190524141054.GB2655@redhat.com> <20190524163310.GG2655@redhat.com> <20190527150409.GA8961@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 29 May 2019 16:57:24 +0000 (UTC) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On 05/28, Deepa Dinamani wrote: > > I agree that signal handller being called and return value not being > altered is an issue with other syscalls also. I was just wondering if > some userspace code assumption would be assuming this. This is not a > kernel bug. > > But, I do not think we have an understanding of what was wrong in > 854a6ed56839a anymore since you pointed out that my assumption was not > correct that the signal handler being called without errno being set > is wrong. Deepa, sorry, I simply can't parse the above... most probably because of my bad English. > One open question: this part of epoll_pwait was already broken before > 854a6ed56839a. Do you agree? > > if (err == -EINTR) { > memcpy(¤t->saved_sigmask, &sigsaved, > sizeof(sigsaved)); > set_restore_sigmask(); > } else > set_current_blocked(&sigsaved); I do not understand why do you think this part was broken :/ > Or, I could revert the signal_pending() check and provide a fix > something like below(not a complete patch) ... > -void restore_user_sigmask(const void __user *usigmask, sigset_t *sigsaved) > +int restore_user_sigmask(const void __user *usigmask, sigset_t > *sigsaved, int sig_pending) > { > > 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 (sig_pending) { > current->saved_sigmask = *sigsaved; > set_restore_sigmask(); > return; > } > > @@ -2330,7 +2330,8 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct > epoll_event __user *, events, > > error = do_epoll_wait(epfd, events, maxevents, timeout); > > - restore_user_sigmask(sigmask, &sigsaved); > + signal_detected = restore_user_sigmask(sigmask, &sigsaved, > error == -EINTR); I fail to understand this pseudo-code, sorry. In particular, do not understand why restore_user_sigmask() needs to return a boolean. The only thing I _seem to_ understand is the "sig_pending" flag passed by the caller which replaces the signal_pending() check. Yes, this is what I think we should do, and this is what I tried to propose from the very beginning in my 1st email in this thread. Oleg.