linux-kernel - Re: [RFC PATCH 1/3] Fix: sched: task_rcu_dereference: check probe_kernel

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-=wg3YyA95bevUaW_fTxmq58ffoHgfFANk-8_RJcGESXEsw@mail.gmail.com>
Date:   Tue, 3 Sep 2019 09:12:43 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Oleg Nesterov <oleg@...hat.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Chris Metcalf <cmetcalf@...hip.com>,
        Christoph Lameter <cl@...ux.com>,
        Kirill Tkhai <tkhai@...dex.ru>, Mike Galbraith <efault@....de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 1/3] Fix: sched: task_rcu_dereference: check
 probe_kernel_address return value

On Tue, Sep 3, 2019 at 9:00 AM Mathieu Desnoyers
<mathieu.desnoyers@...icios.com> wrote:
>
> probe_kernel_address can return -EFAULT on error, which leads to use of
> an uninitialized or partially initialized sighand variable.

I think this comment and this code is actively misleading.

There is no "uninitialized or partially initialized sighand variable".
That's completely wrong.

The sighand variable is always completely initialized. It's just that
the check for "is it initialized" is _not_ the return value from
probe_kernel_address(), because that return value is simply not
sufficient.

So this is just wrong. Don't do it. You're just confusing the issue,
and you're making statments that aren't true in the commit message,
and making the code do a pointless and odd check.

If you want to change this code for legibility, you should just add a
comment above the probe_kernel_address() about why the return value is
ignored, and why the check _below_ that code verifies the value of
sighand with a different check.

                Linus