linux-kernel - Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array

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: <CACT4Y+awYCAwoCkh8PQ-i2BdkxeuFp9meVXp=Yv6hGY8YjDnQg@mail.gmail.com>
Date:   Fri, 24 Mar 2017 14:47:15 +0100
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     "H. Peter Anvin" <hpa@...or.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Michael Davidson <md@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Michal Marek <mmarek@...e.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Shaohua Li <shli@...nel.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "open list:KERNEL BUILD + fi..." <linux-kbuild@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-crypto@...r.kernel.org, linux-raid@...r.kernel.org
Subject: Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array

On Fri, Mar 17, 2017 at 9:04 PM,  <hpa@...or.com> wrote:
> On March 17, 2017 12:27:46 PM PDT, Peter Zijlstra <peterz@...radead.org> wrote:
>>On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
>>> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra
>><peterz@...radead.org> wrote:
>>> >
>>> > Be that as it may; what you construct above is disgusting. Surely
>>the
>>> > code can be refactored to not look like dog vomit?
>>> >
>>> > Also; its not immediately obvious conf->copies is 'small' and this
>>> > doesn't blow up the stack; I feel that deserves a comment
>>somewhere.
>>> >
>>>
>>> I agree that the code is horrible.
>>>
>>> It is, in fact, exactly the same solution that was used to remove
>>> variable length arrays in structs from several of the crypto drivers
>>a
>>> few years ago - see the definition of SHASH_DESC_ON_STACK() in
>>> "crypto/hash.h" - I did not, however, hide the horrors in a macro
>>> preferring to leave the implementation visible as a warning to
>>whoever
>>> might touch the code next.
>>>
>>> I believe that the actual stack usage is exactly the same as it was
>>previously.
>>>
>>> I can certainly wrap this  up in a macro and add comments with
>>> appropriately dire warnings in it if you feel that is both necessary
>>> and sufficient.
>>
>>We got away with ugly in the past, so we should get to do it again?
>
> Seriously, you should have taken the hack the first time that this needs to be fixed.  Just because this is a fairly uncommon construct in the kernel doesn't mean it is not in userspace.


There is a reason why it is fairly uncommon in kernel.
Initially it was used more widely, but then there was a decision to
drop all uses of this feature. Namely:
Linus: "We should definitely drop it. The feature is an abomination".
https://lkml.org/lkml/2013/9/23/500
I really don't understand why you cling onto this last use of the
feature. Having a single use of a compiler extension on an error path
of a non-mandatory driver does not look like a great idea to me. Let's
just kill it off outside of clang discussion.