Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180705161709.17028-1-bart.vanassche@wdc.com>
References: <20180705161709.17028-1-bart.vanassche@wdc.com>
From:   Kees Cook <keescook@chromium.org>
Date:   Sat, 14 Jul 2018 18:49:24 -0700
Message-ID: <CAGXu5jJJLh+oMcZ94nkrwCvyJzk6MzOATvt66WC-rHfxGTJh0Q@mail.gmail.com>
Subject: Re: [PATCH] kernel.h: Avoid that sparse complains about using sizeof(void)
To:     Bart Van Assche <bart.vanassche@wdc.com>,
        Linus Torvalds <torvalds@linux-foundation.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Martin Uecker <Martin.Uecker@med.uni-goettingen.de>,
        Ingo Molnar <mingo@kernel.org>,
        Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
        "# 3.4.x" <stable@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jul 5, 2018 at 9:17 AM, Bart Van Assche <bart.vanassche@wdc.com> wrote:
> The macro __is_constexpr() causes sparse to report the following:
>
>     warning: expression using sizeof(void)
>
> Avoid this by using __builtin_constant_p() instead.
>
> Fixes: 3c8ba0d61d04 ("kernel.h: Retain constant expression output for max()/min()")
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Cc: <stable@vger.kernel.org>
> ---
>  include/linux/kernel.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index d23123238534..a9f0d0d48971 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -811,13 +811,19 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>  #define __typecheck(x, y) \
>                 (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>
> +#ifndef __CHECKER__
>  /*
>   * This returns a constant expression while determining if an argument is
>   * a constant expression, most importantly without evaluating the argument.
> - * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
> + * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>. However, this
> + * macro causes sparse to report the warning "expression using sizeof(void)".
> + * Hence use __builtin_constant_p() instead when using sparse.
>   */
>  #define __is_constexpr(x) \
>         (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
> +#else
> +#define __is_constexpr(x) __builtin_constant_p((x))
> +#endif
>
>  #define __no_side_effects(x, y) \
>                 (__is_constexpr(x) && __is_constexpr(y))

I'm fine with this; it'll only activate for sparse. I'd like to get
Linus's eyes on it, though, since this macro caused us SO much pain
that I'm nervous to change it without some greater level of review. :)

Acked-by: Kees Cook <keescook@chromium.org>

Thanks!

-Kees

-- 
Kees Cook
Pixel Security