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

[David Laight <david.laight.linux@gmail.com>]

On Mon, 12 Jan 2026 14:18:56 -0800
Kees Cook <kees@kernel.org> wrote:

> On Sun, Dec 14, 2025 at 12:58:57PM +0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > 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@gmail.com>
> > ---
> > 
> > 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)

I think you mean 0xFF (and below) to match the old code.
But since your 'r' is 'u8' (but see below) the mask should be discarded
by the compiler anyway.

> > +#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)

'write' is always a constant 0 or 1, and you are writing it to a u8
(which will mask with 0xff anyway).
So you are adding code that just makes it more difficult to read.

> 
> so we're always getting processing a u8?

IIRC that is just passed as a function parameter, not stored in a structure?
If so there isn't any such beast as a 'u8'.
It will always be passed exactly the same way an int is passed.
So the parameter might as well be a u32 (the code might shrink).
By saying it is a u8 you just force the compiler to mask any (non-constant)
calculated value to 8 bits after writing it to the register that holds
the value (and on the function parameter).
All the arithmetic is done after promoting the u8 to 'signed int'.
You won't see the 'gory details' on x86, it and m68k are the only cpu
that have 8/16 bit registers (as part of the 32bit ones).

ISTR there is a check that FUNC isn't too big (because there are no where
near 127 of them), that will pick up any 'accidental garbage' before it
breaks badly - no need to mask the values to 8 bits at all.

	David

> 
> -Kees
>