netdev - Re: [RFC net-next 1/2] overflow: add DECLARE

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] [day] [month] [year] [list]

Message-ID: <6C572B68-2913-4F5B-8295-C001A98CA7A2@kernel.org>
Date: Mon, 07 Aug 2023 07:47:47 -0700
From: Kees Cook <kees@...nel.org>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Alexander Lobakin <aleksander.lobakin@...el.com>
CC: Kees Cook <keescook@...omium.org>, Jacob Keller <jacob.e.keller@...el.com>,
 intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org
Subject: Re: [RFC net-next 1/2] overflow: add DECLARE_FLEX() for on-stack allocs

On August 7, 2023 5:42:31 AM PDT, Przemek Kitszel <przemyslaw.kitszel@...el.com> wrote:
>On 8/4/23 17:44, Alexander Lobakin wrote:
>> From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>> Date: Fri, 4 Aug 2023 15:47:48 +0200
>> 
>>> On 8/2/23 00:31, Kees Cook wrote:
>>> 
>>> [...]
>>> 
>>>> Initially I was struggling to make __counted_by work, but it seems we can
>>>> use an initializer for that member, as long as we don't touch the
>>>> flexible
>>>> array member in the initializer. So we just need to add the counted-by
>>>> member to the macro, and use a union to do the initialization. And if
>>>> we take the address of the union (and not the struct within it), the
>>>> compiler will see the correct object size with __builtin_object_size:
>>>> 
>>>> #define DEFINE_FLEX(type, name, flex, counter, count) \
>>>>       union { \
>>>>           u8   bytes[struct_size_t(type, flex, count)]; \
>>>>           type obj; \
>>>>       } name##_u __aligned(_Alignof(type)) = { .obj.counter = count }; \
>>>>       /* take address of whole union to get the correct
>>>> __builtin_object_size */ \
>>>>       type *name = (type *)&name##_u
>>>> 
>>>> i.e. __builtin_object_size(name, 1) (as used by FORTIFY_SOURCE, etc)
>>>> works correctly here, but breaks (sees a zero-sized flex array member)
>>>> if this macro ends with:
>>>> 
>>>>       type *name = &name##_u.obj
>>> 
>>> __builtin_object_size(name, 0) works fine for both versions (with and
>>> without .obj at the end)
>>> 
>>> however it does not work for builds without -O2 switch, so
>>> struct_size_t() is rather a way to go :/
>> 
>> You only need to care about -O2 and -Os, since only those 2 are
>> officially supported by Kbuild. Did you mean it doesn't work on -Os as well?
>
>Both -Os and -O2 are fine here.
>
>One thing is that perhaps a "user friendly" define for "__builtin_object_size(name, 1)" would avoid any potential for misleading "1" with any "counter" variable, will see.

I meant that FORTIFY_SOURCE will work correctly in the example I gave. (__builtin_object_size() is used there.) Also note that "max size" (mode 0) is unchanged in either case, but mode 1 needs to see the outer object to get the min size correct.

-Kees


-- 
Kees Cook