Re: [PATCH 1/1] Fortify: Use C arithmetic not FIELD_xxx() in FORTIFY_REASON defines

[Kees Cook]

On Sun, Dec 14, 2025 at 12:58:57PM +0000, david.laight.linux@xxxxxxxxx wrote:
> From: David Laight <david.laight.linux@xxxxxxxxx>
> 
> FIELD_GET() and FIELD_PREP() are mainly useful for hardware register
> accesses, but here they are being used for some very simple oprations.
> 
> This wouldn't matter much, but they contain a lot of compile-time
> checks (that really aren't needed here) that bloat the expansion
> of FIELD_GET(GENMASK(7, 1), func) to over 18KB.
> Even with the 'bloat reduced' FIELD_GET/PREP they are still hundreds of
> characters.
> 
> Replace FIELD_GET(BIT(0), r) with ((r) & 1), FIELD_GET(GENMASK(7, 1), r) with
> (r) >> 1), and (FIELD_PREP(BIT(0), write) | FIELD_PREP(GENMASK(7, 1), func))
> with ((func) << 1 | (write)).
> 
> The generated code is the same, but it makes the .c file less obfuctaced,
> the .i file much easier to read, and should marginally decrease compilation
> time.
> 
> Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> ---
> 
> Note that changing 'const u8 reason' to 'const unsigned int reason' generates
> better code - in this case removing 2 instructions (one in each of the called
> functions).
> 
>  include/linux/fortify-string.h | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index b3b53f8c1b28..171982e53c9a 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -2,7 +2,6 @@
>  #ifndef _LINUX_FORTIFY_STRING_H_
>  #define _LINUX_FORTIFY_STRING_H_
>  
> -#include <linux/bitfield.h>
>  #include <linux/bug.h>
>  #include <linux/const.h>
>  #include <linux/limits.h>
> @@ -10,10 +9,9 @@
>  #define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
>  #define __RENAME(x) __asm__(#x)
>  
> -#define FORTIFY_REASON_DIR(r)		FIELD_GET(BIT(0), r)
> -#define FORTIFY_REASON_FUNC(r)		FIELD_GET(GENMASK(7, 1), r)
> -#define FORTIFY_REASON(func, write)	(FIELD_PREP(BIT(0), write) | \
> -					 FIELD_PREP(GENMASK(7, 1), func))
> +#define FORTIFY_REASON_DIR(r)		((r) & 1)
> +#define FORTIFY_REASON_FUNC(r)		((r) >> 1)

Sure, we can do this. I agree, the preprocessor gunk is huge currently.
For the above, how about keeping with the original logic and use:

#define FORTIFY_REASON_FUNC(r)			(((r) & 0xF) >> 1)

> +#define FORTIFY_REASON(func, write)	((func) << 1 | (write))

and:

> +#define FORTIFY_REASON(func, write)	((func) << 1 | (write))

#define FORTIFY_REASON(func, write)	(((func) << 1 | ((write) & 1)) & 0xF)

so we're always getting processing a u8?

-Kees

-- 
Kees Cook