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

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: <383ed0428fd2415aa7ab09255134d61c@AcuMS.aculab.com>
Date: Sun, 15 Dec 2024 19:06:12 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Kees Cook' <kees@...nel.org>, Nathan Chancellor <nathan@...nel.org>
CC: Thomas Weißschuh <linux@...ssschuh.net>, Nilay Shroff
	<nilay@...ux.ibm.com>, Yury Norov <yury.norov@...il.com>, Qing Zhao
	<qing.zhao@...cle.com>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] fortify: Hide run-time copy size from value range
 tracking

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.

	David

> +	__underlying_##op(p, q, __copy_size);				\
>  })
> 
>  /*
> --
> 2.34.1
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)