linux-hardening - Re: [PATCH v2 1/2] Introduce flexible array struct helpers

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: <202210260904.41D0BE00A@keescook>
Date:   Wed, 26 Oct 2022 13:33:47 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Keith Packard <keithp@...thp.com>
Cc:     "Darrick J . Wong" <djwong@...nel.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Francis Laniel <laniel_francis@...vacyrequired.com>,
        Daniel Axtens <dja@...ens.net>,
        Dan Williams <dan.j.williams@...el.com>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        Guenter Roeck <linux@...ck-us.net>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Tadeusz Struk <tadeusz.struk@...aro.org>,
        Zorro Lang <zlang@...hat.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Andy Shevchenko <andriy.shevchenko@...el.com>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org,
        linux-xfs@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2 1/2] Introduce flexible array struct helpers

On Mon, Oct 24, 2022 at 11:35:03AM -0700, Keith Packard wrote:
> Kees Cook <keescook@...omium.org> writes:
> 
> > + * struct flex_array_struct_example {
> > + *	...			 // arbitrary members
> > + *	bounded_flex_array(
> > + *		u16, part_count, // count of elements stored in "parts" below.
> > + *		u32, parts	 // flexible array with elements of type u32.
> > + *	);
> > + * );
> 
> > + * struct flex_array_struct_example {
> > + *	...		// position-sensitive members
> > + *	// count of elements stored in "parts" below.
> > + *	DECLARE_FAS_COUNT(u16, part_count);
> > + *	..		// position-sensitive members
> > + *	// flexible array with elements of type u32.
> > + *	DECLARE_FAS_ARRAY(u32, parts);
> > + * };
> 
> I'm sure there's a good reason, but these two macros appear to be doing
> similar things and yet have very different naming conventions. Maybe:
> 
>         FAS_DECLARE_COUNT(type, name)
>         FAS_DECLARE_ARRAY(type, name)
>         FAS_DECLARE(size_type, size_name, array_type, array_name)

Well, the custom has been for individual things, call it "DECLARE_*",
and for groups, we went with lower-case macros (e.g. struct_group()).

> 
> > +/* For use with flexible array structure helpers, in <linux/flex_array.h> */
> > +#define __DECLARE_FAS_COUNT(TYPE, NAME)					\
> > +	union {								\
> > +		TYPE __flex_array_elements_count;			\
> > +		TYPE NAME;						\
> > +	}
> 
> How often could that second "public" member be 'const'? That would catch
> places which accidentally assign to this field.
> 
> For code which does want to write to this field, is it mostly trimming
> data from the end, or does it actually smash in arbitrary values? For
> the former case, would it be helpful to have a test to make sure the
> assigned size isn't larger than the real size (yeah, that would probably
> take an extra field holding the real size), or larger than the current size?

I don't think this'll work within arbitrary struct declarations, but it
would be nice. :)

-- 
Kees Cook