[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 15 Mar 2018 15:16:51 -0700
From: Kees Cook <keescook@...omium.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Josh Poimboeuf <jpoimboe@...hat.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>, Randy Dunlap <rdunlap@...radead.org>,
Miguel Ojeda <miguel.ojeda.sandonis@...il.com>, Ingo Molnar <mingo@...nel.org>,
David Laight <David.Laight@...lab.com>, Ian Abbott <abbotti@....co.uk>,
linux-input <linux-input@...r.kernel.org>, linux-btrfs <linux-btrfs@...r.kernel.org>,
Network Development <netdev@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH v4 1/2] kernel.h: Introduce const_max() for VLA removal
On Thu, Mar 15, 2018 at 2:42 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Thu, Mar 15, 2018 at 12:47 PM, Kees Cook <keescook@...omium.org> wrote:
>>
>> To gain the ability to compare differing types, the arguments are
>> explicitly cast to size_t.
>
> Ugh, I really hate this.
>
> It silently does insane things if you do
>
> const_max(-1,6)
>
> and there is nothing in the name that implies that you can't use
> negative constants.
Yeah, I didn't like that effect either. I was seeing this:
./include/linux/kernel.h:836:14: warning: comparison between ‘enum
<anonymous>’ and ‘enum <anonymous>’ [-Wenum-compare]
(x) > (y) ? \
^
./include/linux/kernel.h:838:7: note: in definition of macro ‘const_max’
(y), \
^
net/ipv6/proc.c:34:11: note: in expansion of macro ‘const_max’
const_max(IPSTATS_MIB_MAX, ICMP_MIB_MAX))
^~~~~~~~~
But it turns out that just doing a typeof() fixes this, and there's no
need for the hard cast to size_t:
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?
-Kees
--
Kees Cook
Pixel Security
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
