linux-kernel - Re: [PATCH] signal: remove the wrong signal_pending() check in restore_user

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]

Message-ID: <CAHk-=wjSOh5zmApq2qsNjmY-GMn4CWe9YwdcKPjT+nVoGiDKOQ@mail.gmail.com>
Date:   Tue, 4 Jun 2019 14:26:42 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Deepa Dinamani <deepa.kernel@...il.com>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Davidlohr Bueso <dbueso@...e.de>, Jens Axboe <axboe@...nel.dk>,
        Davidlohr Bueso <dave@...olabs.net>, e@...24.org,
        Jason Baron <jbaron@...mai.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-aio@...ck.org, omar.kilani@...il.com,
        Thomas Gleixner <tglx@...utronix.de>,
        stable <stable@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        David Laight <David.Laight@...lab.com>
Subject: Re: [PATCH] signal: remove the wrong signal_pending() check in restore_user_sigmask()

On Tue, Jun 4, 2019 at 6:41 AM Oleg Nesterov <oleg@...hat.com> wrote:
>
> This is the minimal fix for stable, I'll send cleanups later.

Ugh. I htink this is correct, but I wish we had a better and more
intuitive interface.

In particular, since restore_user_sigmask() basically wants to check
for "signal_pending()" anyway (to decide if the mask should be
restored by signal handling or by that function), I really get the
feeling that a lot of these patterns like

> -       restore_user_sigmask(ksig.sigmask, &sigsaved);
> -       if (signal_pending(current) && !ret)
> +
> +       interrupted = signal_pending(current);
> +       restore_user_sigmask(ksig.sigmask, &sigsaved, interrupted);
> +       if (interrupted && !ret)
>                 ret = -ERESTARTNOHAND;

are wrong to begin with, and we really should aim for an interface
which says "tell me whether you completed the system call, and I'll
give you an error return if not".

How about we make restore_user_sigmask() take two return codes: the
'ret' we already have, and the return we would get if there is a
signal pending and w're currently returning zero.

IOW, I think the above could become

        ret = restore_user_sigmask(ksig.sigmask, &sigsaved, ret, -ERESTARTHAND);

instead if we just made the right interface decision.

Hmm?

             Linus