Re: [PATCH v2] fortify: Hide run-time copy size from value range tracking

[Kees Cook <kees@kernel.org>]

On Sun, Dec 15, 2024 at 07:06:12PM +0000, David Laight wrote:
> From: Kees Cook
> > Sent: 14 December 2024 01:36
> ...
> > In order to silence this false positive but keep deterministic
> > compile-time warnings intact, hide the length variable from GCC with
> > OPTIMIZE_HIDE_VAR() before calling the builtin memcpy.
> ...
> > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> > index 0d99bf11d260..1eef0119671c 100644
> > --- a/include/linux/fortify-string.h
> > +++ b/include/linux/fortify-string.h
> > @@ -616,6 +616,12 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
> >  	return false;
> >  }
> > 
> > +/*
> > + * To work around what seems to be an optimizer bug, the macro arguments
> > + * need to have const copies or the values end up changed by the time they
> > + * reach fortify_warn_once(). See commit 6f7630b1b5bc ("fortify: Capture
> > + * __bos() results in const temp vars") for more details.
> > + */
> >  #define __fortify_memcpy_chk(p, q, size, p_size, q_size,		\
> >  			     p_size_field, q_size_field, op) ({		\
> >  	const size_t __fortify_size = (size_t)(size);			\
> > @@ -623,6 +629,8 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
> >  	const size_t __q_size = (q_size);				\
> >  	const size_t __p_size_field = (p_size_field);			\
> >  	const size_t __q_size_field = (q_size_field);			\
> > +	/* Keep a mutable version of the size for the final copy. */	\
> > +	size_t __copy_size = __fortify_size;				\
> >  	fortify_warn_once(fortify_memcpy_chk(__fortify_size, __p_size,	\
> >  				     __q_size, __p_size_field,		\
> >  				     __q_size_field, FORTIFY_FUNC_ ##op), \
> > @@ -630,7 +638,11 @@ __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);			\
> > +	/* Hide only the run-time size from value range tracking to */	\
> > +	/* silence compile-time false positive bounds warnings. */	\
> > +	if (!__builtin_constant_p(__fortify_size))			\
> > +		OPTIMIZER_HIDE_VAR(__copy_size);			\
> 
> I think you can make that:
> 	if (!__builtin_constant_p(__copy_size)) \
> 		OPTIMISER_HIDE_VAR(__copy_size) \
> which is probably more readable.

Yeah, that tests out fine. I've updated it locally. Thanks!

-Kees

-- 
Kees Cook