Re: [PATCH v3 1/3] string: Make stpcpy() possible to use

[Kees Cook]

On Wed, Jan 26, 2022 at 08:12:22PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 26, 2022 at 09:49:38AM -0800, Nick Desaulniers wrote:
> > On Wed, Jan 26, 2022 at 6:19 AM Andy Shevchenko
> > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > >
> > > It is a good rule to avoid submitting code without users.
> > > Currently the stpcpy() is unusable due to missed declaration.
> > > Any attempts to use it will bring something like:
> > >
> > >   error: implicit declaration of function ‘stpcpy’ [-Werror=implicit-function-declaration]
> > >
> > > Move declaration to the header and guard it as other string functions.
> 
> ...
> 
> > Recall the discussion from Kees:
> > https://lore.kernel.org/lkml/CAK7LNAQXo5-5W6hvNMEVPBPf3tRWaf-pQdSR-0OHyi4RCGhjsQ@xxxxxxxxxxxxxx/
> > and
> > https://lore.kernel.org/lkml/202008150921.B70721A359@keescook/
> 
> For the record :-)
> https://lore.kernel.org/lkml/CAHp75VfniSw3AFTyyDk2OoAChGx7S6wF7sZKpJXNHmk97BoRXA@xxxxxxxxxxxxxx/
> [...
> strcpy() is not a bad API for the cases when you know what you are
> doing. A problem that most of the developers do not know what they are
> doing.
> No need to split everything to bad and good by its name or semantics,
> each API has its own pros and cons and programmers must use their
> brains.
> ...]

Developers should not need to remember to avoid foot-guns; the toolchain
should be doing all of that. The trouble is that C (and its standard
libs) are filled with foot-guns.

I do not want to add another foot-gun API to the kernel; we've been
working very hard to _remove_ them. :) If the kernel's stpcpy() _only_
worked on all known-size strings, etc, so that memory safety could be
determined at compile-time, then I'd have no objection.

What's not clear to me is if such macro versions would be workable for
the reason stpcpy() was added in the first place, which was the compiler
transforming other calls stuff into library calls it thinks are defined.

Totally untested:

#define stpcpy(dst, src) ({					\
	size_t _stp__dst = __builtin_object_size(dst, 1);	\
	size_t _stp__src = __builtin_object_size(src, 1);	\
								\
	BUILD_BUG_ON(_stp__dst == -1 || _stp__src == -1);	\
	BUILD_BUG_ON(_stp__src > _stp__dst);			\
								\
	__builtin_stpcpy(dst, src);				\
})

(Is there even a __builtin_stpcpy()?)

-- 
Kees Cook