linux-kernel - Re: [RFC PATCH] membarrier: handle nohz_full with expedited thread registration

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: <CA+55aFz01xaATut1r_FmzB7v39JngjjydqmKHivy+-5a1bbSKg@mail.gmail.com>
Date:   Mon, 16 Jan 2017 12:15:22 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     "Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Josh Triplett <josh@...htriplett.org>,
        KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Nicholas Miell <nmiell@...cast.net>,
        Ingo Molnar <mingo@...hat.com>,
        Alan Cox <gnomes@...rguk.ukuu.org.uk>,
        Lai Jiangshan <laijs@...fujitsu.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        David Howells <dhowells@...hat.com>,
        Pranith Kumar <bobby.prani@...il.com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Shuah Khan <shuahkh@....samsung.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [RFC PATCH] membarrier: handle nohz_full with expedited thread registration

Excuse my french, but this looks like incredible shit to me.

On Mon, Jan 16, 2017 at 11:51 AM, Mathieu Desnoyers
<mathieu.desnoyers@...icios.com> wrote:
> +
> +static int membarrier_register_expedited(struct task_struct *t)
> +{
> +       struct rq *rq;
> +
> +       if (t->membarrier_expedited == UINT_MAX)
> +               return -EOVERFLOW;
> +       rq = this_rq();
> +       raw_spin_lock(&rq->lock);
> +       t->membarrier_expedited++;
> +       raw_spin_unlock(&rq->lock);
> +       return 0;
> +}

Yeah, taking the rq lock with

 (a) a random "rq" that isn't stable

 (b) without disabling interrupts

 (c) using an internal scheduler helper that isn't supposed to be used
externally

 (d) when it doesn't even make any sense in the first place for a
per-thread value that is never modified by any other threads!

 (e) .. and you expose this ABSOLUTELY SHIT as a random system call.

Oh, and the clone semantics make no sense either.

In fact, it just makes me doubt everything about the whole membarrier
concept, because it appears *so* terminally broken.

So unless I'm seriously missing something, this is just about the
worst piece of code I have seen this year.

No.

NO NO NO.

It really smells so broken that I'm wondering if I'm missing anything.
But I don't think I am. I think the code is just pure garbage.

                  Linus