linux-kernel - Re: [PATCH v9 2/8] util_macros: Add exact_type macro to catch type mis-match while compiling

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: <202208250953.A71FEE45E@keescook>
Date:   Thu, 25 Aug 2022 10:19:23 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Gwan-gyeong Mun <gwan-gyeong.mun@...el.com>
Cc:     intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, mchehab@...nel.org,
        chris@...is-wilson.co.uk, matthew.auld@...el.com,
        thomas.hellstrom@...ux.intel.com, jani.nikula@...el.com,
        nirmoy.das@...el.com, airlied@...ux.ie, daniel@...ll.ch,
        andi.shyti@...ux.intel.com, andrzej.hajda@...el.com,
        mauro.chehab@...ux.intel.com,
        intel-gfx-trybot@...ts.freedesktop.org
Subject: Re: [PATCH v9 2/8] util_macros: Add exact_type macro to catch type
 mis-match while compiling

On Wed, Aug 24, 2022 at 05:45:08PM +0900, Gwan-gyeong Mun wrote:
> It adds exact_type and exactly_pgoff_t macro to catch type mis-match while
> compiling. The existing typecheck() macro outputs build warnings, but the
> newly added exact_type() macro uses the BUILD_BUG_ON() macro to generate
> a build break when the types are different and can be used to detect
> explicit build errors.
> 
> v6: Move macro addition location so that it can be used by other than drm
>     subsystem (Jani, Mauro, Andi)
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@...el.com>
> Cc: Thomas Hellström <thomas.hellstrom@...ux.intel.com>
> Cc: Matthew Auld <matthew.auld@...el.com>
> Cc: Nirmoy Das <nirmoy.das@...el.com>
> Cc: Jani Nikula <jani.nikula@...el.com>
> Cc: Andi Shyti <andi.shyti@...ux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@...nel.org>
> ---
>  include/linux/util_macros.h | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
> index 72299f261b25..b6624b275257 100644
> --- a/include/linux/util_macros.h
> +++ b/include/linux/util_macros.h
> @@ -2,6 +2,9 @@
>  #ifndef _LINUX_HELPER_MACROS_H_
>  #define _LINUX_HELPER_MACROS_H_
>  
> +#include <linux/types.h>
> +#include <linux/bug.h>
> +
>  #define __find_closest(x, a, as, op)					\
>  ({									\
>  	typeof(as) __fc_i, __fc_as = (as) - 1;				\
> @@ -38,4 +41,26 @@
>   */
>  #define find_closest_descending(x, a, as) __find_closest(x, a, as, >=)
>  
> +/**
> + * exact_type - break compile if source type and destination value's type are
> + * not the same
> + * @T: Source type
> + * @n: Destination value
> + *
> + * It is a helper macro for a poor man's -Wconversion: only allow variables of
> + * an exact type. It determines whether the source type and destination value's
> + * type are the same while compiling, and it breaks compile if two types are
> + * not the same
> + */
> +#define exact_type(T, n) \
> +	BUILD_BUG_ON(!__builtin_constant_p(n) && !__builtin_types_compatible_p(T, typeof(n)))

Maybe use __same_type() here instead of open-coded
__builtin_types_compatible_p()? Also, IIUC, currently coding style
advise is to use _Static_assert when possible over BUILD_BUG_ON for
error message readability.

This macro has a trap-door for literals, yes?
i.e.  exact_type(pgoff_t, 5) will pass?

I also note that this is very close to the really common (and open-coded)
test scattered around the kernel already (BUILD_BUG_ON(__same_type(a,
b))), so I think it's good to get a macro defined for it, though I'm not
sure about the trap door test. Regardless, I'd like to bikeshed the name
a bit; I think this should be named something a bit more clear about
what happens on failure. Perhaps: assert_type()? Or to capture the
trapdoor idea, assert_typable()?

#define assert_type(t1, t2)	_Static_assert(__same_type(t1, t2))
#define assert_typable(t, n)	_Static_assert(__builtin_constant_p(n) ||
					       __same_type(t, typeof(n))

> +
> +/**
> + * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
> + * @n: value to compare pgoff_t type
> + *
> + * It breaks compile if the argument value's type is not pgoff_t type.
> + */
> +#define exactly_pgoff_t(n) exact_type(pgoff_t, n)

Why specialize this? Just use assert_typable(pgoff_t, n) in the other
patches? It's almost the same amount to write. :)

-- 
Kees Cook