linux-kernel - Re: pselect/etc semantics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 30 May 2019 09:59:08 -0700
From:   Deepa Dinamani <deepa.kernel@...il.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Oleg Nesterov <oleg@...hat.com>, Al Viro <viro@...iv.linux.org.uk>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>, dbueso@...e.de, axboe@...nel.dk,
        Davidlohr Bueso <dave@...olabs.net>, Eric Wong <e@...24.org>,
        Jason Baron <jbaron@...mai.com>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        linux-aio <linux-aio@...ck.org>,
        Omar Kilani <omar.kilani@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>, stable@...r.kernel.org
Subject: Re: pselect/etc semantics

On Thu, May 30, 2019 at 8:48 AM Deepa Dinamani <deepa.kernel@...il.com> wrote:
>
> > On May 30, 2019, at 8:38 AM, Eric W. Biederman <ebiederm@...ssion.com> wrote:
> >
> > ebiederm@...ssion.com (Eric W. Biederman) writes:
> >
> >> Which means I believe we have a semantically valid change in behavior
> >> that is causing a regression.
> >
> > I haven't made a survey of all of the functions yet but
> > fucntions return -ENORESTARTNOHAND will never return -EINTR and are
> > immune from this problem.
> >
> > AKA pselect is fine.  While epoll_pwait can be affected.
>
> This was my understanding as well.

I think I was misremembered here. I had noted this before:
https://lore.kernel.org/linux-fsdevel/CABeXuvq7gCV2qPOo+Q8jvNyRaTvhkRLRbnL_oJ-AuK7Sp=P3QQ@mail.gmail.com/

"sys_io_pgetevents() does not seem to have this problem as we are still
checking signal_pending() here.
sys_pselect6() seems to have a similar problem. The changes to
sys_pselect6() also impact sys_select() as the changes are in the
common code path."

This was the code replaced for io_pgetevents by 854a6ed56839a40f6b is as below.
No matter what events completed, there was signal_pending() check
after the return from do_io_getevents().

--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2110,18 +2110,9 @@ SYSCALL_DEFINE6(io_pgetevents,
                return ret;

        ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
-       if (signal_pending(current)) {
-               if (ksig.sigmask) {
-                       current->saved_sigmask = sigsaved;
-                       set_restore_sigmask();
-               }
-
-               if (!ret)
-                       ret = -ERESTARTNOHAND;
-       } else {
-               if (ksig.sigmask)
-                       sigprocmask(SIG_SETMASK, &sigsaved, NULL);
-       }
+       restore_user_sigmask(ksig.sigmask, &sigsaved);
+       if (signal_pending(current) && !ret)
+               ret = -ERESTARTNOHAND;

Can I ask a simple question for my understanding?

man page for epoll_pwait says

EINTR
The call was interrupted by a signal handler before either any of the
requested events occurred or the timeout expired; see signal(7).

But it is not clear to me if we can figure out(without race) the
chronological order if one of the requested events are completed or a
signal came first.
Is this a correct exectation?

Also like pointed out above, this behavior is not consistent for all
such syscalls(io_pgetevents). Was this also by design?

-Deepa