All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Randy Dunlap <rdunlap@infradead.org>,
	Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
	Ingo Molnar <mingo@kernel.org>,
	David Laight <David.Laight@aculab.com>,
	Ian Abbott <abbotti@mev.co.uk>,
	linux-input <linux-input@vger.kernel.org>,
	linux-btrfs <linux-btrfs@vger.kernel.org>,
	Network Development <netdev@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Kernel Hardening <kernel-hardening@lists.openwall.com>
Subject: Re: [PATCH v4 1/2] kernel.h: Introduce const_max() for VLA removal
Date: Thu, 15 Mar 2018 15:46:21 -0700	[thread overview]
Message-ID: <CAGXu5jLHam5kz18k9uvhsz2WYodkF2v1tsEOV4Sx0O7jir4B3A@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFwDJ906oQ-98L2DNrjfKtb6cd5ykwMxpG942qxCFmAoEQ@mail.gmail.com>

On Thu, Mar 15, 2018 at 3:23 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Mar 15, 2018 at 3:16 PM, Kees Cook <keescook@chromium.org> wrote:
>>
>> size_t __error_not_const_arg(void) \
>> __compiletime_error("const_max() used with non-compile-time constant arg");
>> #define const_max(x, y)                                         \
>>         __builtin_choose_expr(__builtin_constant_p(x) &&        \
>>                               __builtin_constant_p(y),          \
>>                               (typeof(x))(x) > (typeof(y))(y) ? \
>>                                         (x) : (y),              \
>>                               __error_not_const_arg())
>>
>> Is typeof() forcing enums to int? Regardless, I'll put this through
>> larger testing. How does that look?
>
> Ok, that alleviates my worry about one class of insane behavior, but
> it does raise a few other questions:
>
>  - what drugs is gcc on where (typeof(x)(x)) makes a difference? Funky.

Yeah, that's why I didn't even try that originally. But in looking
back at max() again, it seemed to be the only thing missing that would
handle the enum evaluation, which turned out to be true.

>  - this does have the usual "what happen if you do
>
>      const_max(-1,sizeof(x))
>
> where the comparison will now be done in 'size_t', and -1 ends up
> being a very very big unsigned integer.
>
> Is there no way to get that type checking inserted? Maybe now is a
> good point for that __builtin_types_compatible(), and add it to the
> constness checking (and change the name of that error case function)?

So, AIUI, I can either get strict type checking, in which case, this
is rejected (which I assume there is still a desire to have):

int foo[const_max(6, sizeof(whatever))];

due to __builtin_types_compatible_p() rejecting it, or I can construct
a "positive arguments only" test, in which the above is accepted, but
this is rejected:

int foo[const_max(-1, sizeof(whatever))];

By using this eye-bleed:

size_t __error_not_const_arg(void) \
__compiletime_error("const_max() used with non-compile-time constant arg");
size_t __error_not_positive_arg(void) \
__compiletime_error("const_max() used with negative arg");
#define const_max(x, y)                                                 \
        __builtin_choose_expr(__builtin_constant_p(x) &&                \
                              __builtin_constant_p(y),                  \
                __builtin_choose_expr((x) >= 0 && (y) >= 0,             \
                                      (typeof(x))(x) > (typeof(y))(y) ? \
                                        (x) : (y),                      \
                                      __error_not_positive_arg()),      \
                __error_not_const_arg())

-Kees

-- 
Kees Cook
Pixel Security

  reply	other threads:[~2018-03-15 22:46 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-15 19:47 [PATCH v4 0/2] Remove false-positive VLAs when using max() Kees Cook
2018-03-15 19:47 ` [PATCH v4 1/2] kernel.h: Introduce const_max() for VLA removal Kees Cook
2018-03-15 21:42   ` Linus Torvalds
2018-03-15 22:16     ` Kees Cook
2018-03-15 22:23       ` Linus Torvalds
2018-03-15 22:46         ` Kees Cook [this message]
2018-03-15 22:58           ` Miguel Ojeda
2018-03-15 23:08             ` Miguel Ojeda
2018-03-15 23:17               ` Miguel Ojeda
2018-03-15 23:31                 ` Kees Cook
2018-03-15 23:34           ` Linus Torvalds
2018-03-15 23:41             ` Kees Cook
2018-03-15 23:46               ` Linus Torvalds
2018-03-15 23:47                 ` Linus Torvalds
2018-03-15 23:49                 ` Kees Cook
2018-03-16  3:05                   ` Miguel Ojeda
2018-03-16 14:15                 ` Rasmus Villemoes
2018-03-15 19:47 ` [PATCH v4 2/2] Remove false-positive VLAs when using max() Kees Cook
2018-03-16  7:52   ` Nikolay Borisov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGXu5jLHam5kz18k9uvhsz2WYodkF2v1tsEOV4Sx0O7jir4B3A@mail.gmail.com \
    --to=keescook@chromium.org \
    --cc=David.Laight@aculab.com \
    --cc=abbotti@mev.co.uk \
    --cc=akpm@linux-foundation.org \
    --cc=jpoimboe@redhat.com \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=mingo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.