linux-hardening - Re: [PATCH] string: Check for "nonstring" attribute on strscpy() arguments

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: <202408221701.62773B4E1@keescook>
Date: Thu, 22 Aug 2024 17:04:08 -0700
From: Kees Cook <kees@...nel.org>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: Justin Stitt <justinstitt@...gle.com>,
	Andy Shevchenko <andy@...nel.org>,
	Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
	Nick Desaulniers <ndesaulniers@...gle.com>,
	Miguel Ojeda <ojeda@...nel.org>, Marco Elver <elver@...gle.com>,
	Nathan Chancellor <nathan@...nel.org>, Hao Luo <haoluo@...gle.com>,
	linux-sparse@...r.kernel.org, linux-hardening@...r.kernel.org,
	Bill Wendling <morbo@...gle.com>,
	Mark Rutland <mark.rutland@....com>,
	Jakub Kicinski <kuba@...nel.org>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	Tony Ambardar <tony.ambardar@...il.com>,
	Petr Pavlu <petr.pavlu@...e.com>, linux-kernel@...r.kernel.org,
	llvm@...ts.linux.dev
Subject: Re: [PATCH] string: Check for "nonstring" attribute on strscpy()
 arguments

On Tue, Aug 06, 2024 at 12:29:30PM +0200, Przemek Kitszel wrote:
> On 8/5/24 23:43, Kees Cook wrote:
> > GCC already checks for arguments that are marked with the "nonstring"[1]
> > attribute when used on standard C String API functions (e.g. strcpy). Gain
> > this compile-time checking also for the kernel's primary string copying
> > function, strscpy().
> > 
> > Note that Clang has neither "nonstring" nor __builtin_has_attribute().
> > 
> > Link: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-nonstring-variable-attribute [1]
> > Signed-off-by: Kees Cook <kees@...nel.org>
> 
> 
> 
> > diff --git a/include/linux/string.h b/include/linux/string.h
> > index 9edace076ddb..95b3fc308f4f 100644
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -76,12 +76,16 @@ ssize_t sized_strscpy(char *, const char *, size_t);
> >    * known size.
> >    */
> >   #define __strscpy0(dst, src, ...)	\
> > -	sized_strscpy(dst, src, sizeof(dst) + __must_be_array(dst))
> > -#define __strscpy1(dst, src, size)	sized_strscpy(dst, src, size)
> > +	sized_strscpy(dst, src, sizeof(dst) + __must_be_array(dst) +	\
> > +				__must_be_cstr(dst) + __must_be_cstr(src))
> > +#define __strscpy1(dst, src, size)	\
> > +	sized_strscpy(dst, src, size + __must_be_cstr(dst) + __must_be_cstr(src))
> >   #define __strscpy_pad0(dst, src, ...)	\
> > -	sized_strscpy_pad(dst, src, sizeof(dst) + __must_be_array(dst))
> > -#define __strscpy_pad1(dst, src, size)	sized_strscpy_pad(dst, src, size)
> > +	sized_strscpy_pad(dst, src, sizeof(dst) + __must_be_array(dst) +	\
> > +				    __must_be_cstr(dst) + __must_be_cstr(src))
> > +#define __strscpy_pad1(dst, src, size)	\
> > +	sized_strscpy_pad(dst, src, size + __must_be_cstr(dst) + __must_be_cstr(src))
> 
> any way to avoid the usual caveat of repeating macro argument?
> 
> a variant of BUILD_BUG that is checking argument and otherwise pasting
> it would nail it, but I didn't pondered how to implement such

The use of __must_be_cstr() shouldn't cause side-effects, so from what I
can tell this is all okay. And since BUILD_BUG_ON_ZERO() resolves to a
constant expression, it shouldn't change the processing of the "size"
argument in the strscpy internals...

-- 
Kees Cook