Re: [PATCHv3] gcc: disable '-Wstrignop-overread' universally for gcc-13+ and FORTIFY_SOURCE

[Kees Cook]

On Thu, Dec 12, 2024 at 10:24:36AM -0800, Kees Cook wrote:
> Or we could unconditionally put the OPTIMIZER_HIDE_VAR() inside
> bitmap_copy() itself:
> 
> 
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index 262b6596eca5..5503ccabe05a 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -251,12 +251,14 @@ static __always_inline void bitmap_fill(unsigned long *dst, unsigned int nbits)
>  static __always_inline
>  void bitmap_copy(unsigned long *dst, const unsigned long *src, unsigned int nbits)
>  {
> -	unsigned int len = bitmap_size(nbits);
> -
> -	if (small_const_nbits(nbits))
> +	if (small_const_nbits(nbits)) {
>  		*dst = *src;
> -	else
> +	} else {
> +		unsigned int len = bitmap_size(nbits);
> +
> +		OPTIMIZER_HIDE_VAR(len);
>  		memcpy(dst, src, len);
> +	}
>  }
>  
>  /*
> 
> I prefer any of these to doing the build-system disabling of the
> warning.

Actually, this should probably be done in the FORTIFY macro instead --
it's what actually tripping the GCC warning since it is the code that is
gleefully issuing a warning and then continuing with a overflowing copy
anyway...


diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index 0d99bf11d260..7203acfb9f17 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -630,7 +630,13 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
 		  __fortify_size,					\
 		  "field \"" #p "\" at " FILE_LINE,			\
 		  __p_size_field);					\
-	__underlying_##op(p, q, __fortify_size);			\
+	if (__builtin_constant_p(__fortify_size)) {			\
+		__underlying_##op(p, q, __fortify_size);		\
+	} else {							\
+		size_t ___fortify_size = __fortify_size;		\
+		OPTIMIZER_HIDE_VAR(___fortify_size);			\
+		__underlying_##op(p, q, ___fortify_size);		\
+	}								\
 })
 
 /*

-- 
Kees Cook