linux-kernel - Re: [PATCH] key: Convert big

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: <CAGXu5jLYLeYWm9GKMm2_5iT4PjaXCi5qsGnZQ2JqFT0DVvmYog@mail.gmail.com>
Date:   Mon, 8 May 2017 15:26:01 -0700
From:   Kees Cook <keescook@...omium.org>
To:     David Howells <dhowells@...hat.com>
Cc:     James Morris <james.l.morris@...cle.com>,
        "Serge E. Hallyn" <serge@...lyn.com>, keyrings@...r.kernel.org,
        linux-security-module <linux-security-module@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] key: Convert big_key payload.data to struct

On Mon, May 8, 2017 at 3:00 PM, David Howells <dhowells@...hat.com> wrote:
> Kees Cook <keescook@...omium.org> 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