Re: [PATCH] key: Convert big_key payload.data to struct
From: Kees Cook
Date: Mon May 08 2017 - 18:26:07 EST
On Mon, May 8, 2017 at 3:00 PM, David Howells <dhowells@xxxxxxxxxx> wrote:
> Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
>> There is a lot of needless casting happening in the big_key data payload.
>> This is harder to trivially verify by static analysis and specifically
>> the randstruct GCC plugin (which was unhappy about casting a struct
>> path across two entries of a void * array). This converts the payload to
>> the actually used structures (one pointer, one embedded struct, and one
>> size_t).
>
> I'd really rather not do this as this moves the definition of an individual
> key type into the general structure (I know I've done this for the keyring
> type, but that's a special part of the keyring code). That's the start of the
> slippery slope into moving all of them in there.
>
> I'd rather you defined, say:
>
> struct big_key_payload {
> u8 *key_data;
> struct path key_path;
> size_t key_len;
> };
>
> in big_key.c and cast &key->payload to it.
This doesn't protect you against changes in struct path size,
though... the existing code (and this proposal) will break if that
ever happens...
What's the problem with defining the types at the top level? That
seems like a nice place to see them all at once.
-Kees
--
Kees Cook
Pixel Security