Re: [PATCH v4 1/2] kernel.h: Introduce const_max() for VLA removal

[Kees Cook]

On Thu, Mar 15, 2018 at 2:42 PM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Mar 15, 2018 at 12:47 PM, Kees Cook <keescook@xxxxxxxxxxxx> 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