netdev - Re: [PATCH] treewide: Replace zero-length arrays with flexible-array member

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: <88e09425-8207-7a1e-8802-886f9694a37f@embeddedor.com>
Date:   Tue, 11 Feb 2020 13:54:22 -0600
From:   "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To:     Greg KH <gregkh@...uxfoundation.org>,
        Kees Cook <keescook@...omium.org>
Cc:     linux-kernel@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        linux-crypto@...r.kernel.org, netdev@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: [PATCH] treewide: Replace zero-length arrays with flexible-array
 member



On 2/11/20 13:38, Greg KH wrote:
> On Tue, Feb 11, 2020 at 11:32:04AM -0800, Kees Cook wrote:
>> On Tue, Feb 11, 2020 at 01:20:36PM -0600, Gustavo A. R. Silva wrote:
>>>
>>>
>>> On 2/11/20 12:32, Greg KH wrote:
>>>> On Tue, Feb 11, 2020 at 11:41:26AM -0600, Gustavo A. R. Silva wrote:
>>>>> The current codebase makes use of the zero-length array language
>>>>> extension to the C90 standard, but the preferred mechanism to declare
>>>>> variable-length types such as these ones is a flexible array member[1][2],
>>>>> introduced in C99:
>>>>>
>>>>> struct foo {
>>>>>         int stuff;
>>>>>         struct boo array[];
>>>>> };
>>>>>
>>>>> By making use of the mechanism above, we will get a compiler warning
>>>>> in case the flexible array does not occur last in the structure, which
>>>>> will help us prevent some kind of undefined behavior bugs from being
>>>>> unadvertenly introduced[3] to the codebase from now on.
>>>>>
>>>>> All these instances of code were found with the help of the following
>>>>> Coccinelle script:
>>>>>
>>>>> @@
>>>>> identifier S, member, array;
>>>>> type T1, T2;
>>>>> @@
>>>>>
>>>>> struct S {
>>>>>   ...
>>>>>   T1 member;
>>>>>   T2 array[
>>>>> - 0
>>>>>   ];
>>>>> };
>>>>>
>>>>> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
>>>>> [2] https://github.com/KSPP/linux/issues/21
>>>>> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>>>>>
>>>>> NOTE: I'll carry this in my -next tree for the v5.6 merge window.
>>>>
>>>> Why not carve this up into per-subsystem patches so that we can apply
>>>> them to our 5.7-rc1 trees and then you submit the "remaining" that don't
>>>> somehow get merged at that timeframe for 5.7-rc2?
>>>>
>>>
>>> Yep, sounds good. I'll do that.
>>
>> FWIW, I'd just like to point out that since this is a mechanical change
>> with no code generation differences (unlike the pre-C90 1-byte array
>> conversions), it's a way better use of everyone's time to just splat
>> this in all at once.
>>
>> That said, it looks like Gustavo is up for it, but I'd like us to
>> generally consider these kinds of mechanical changes as being easier to
>> manage in a single patch. (Though getting Acks tends to be a bit
>> harder...)
> 
> Hey, if this is such a mechanical patch, let's get it to Linus now,
> what's preventing that from being merged now?
> 

Well, the only thing is that this has never been in linux-next.

Thanks
--
Gustavo