On Sat, Jul 06, 2019 at 02:42:04PM +0200, Stephen Kitt wrote: > On Tue, 2 Jul 2019 10:25:04 -0700, Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > On Sat, Jun 29, 2019 at 06:15:37PM +0200, Stephen Kitt wrote: > > > On Fri, 28 Jun 2019 17:25:48 +0530, Nitin Gote <nitin.r.gote@xxxxxxxxx> > > > wrote: > > > > 1. Deprecate strcpy() in favor of strscpy(). > > > > > > This isn’t a comment “against” this patch, but something I’ve been > > > wondering recently and which raises a question about how to handle > > > strcpy’s deprecation in particular. There is still one scenario where > > > strcpy is useful: when GCC replaces it with its builtin, inline version... > > > > > > Would it be worth introducing a macro for strcpy-from-constant-string, > > > which would check that GCC’s builtin is being used (when building with > > > GCC), and fall back to strscpy otherwise? > > > > How would you suggest it operate? A separate API, or something like the > > existing overloaded strcpy() macros in string.h? > > The latter; in my mind the point is to simplify the thought process for > developers, so strscpy should be the “obvious” choice in all cases, even when > dealing with constant strings in hot paths. Something like > > __FORTIFY_INLINE ssize_t strscpy(char *dest, const char *src, size_t count) > { > size_t dest_size = __builtin_object_size(dest, 0); > size_t src_size = __builtin_object_size(src, 0); > if (__builtin_constant_p(count) && > __builtin_constant_p(src_size) && > __builtin_constant_p(dest_size) && > src_size <= count && > src_size <= dest_size && > src[src_size - 1] == '\0') { > strcpy(dest, src); > return src_size - 1; > } else { > return __strscpy(dest, src, count); > } > } > > with the current strscpy renamed to __strscpy. I imagine it’s not necessary > to tie this to FORTIFY — __OPTIMIZE__ should be sufficient, shouldn’t it? > Although building on top of the fortified strcpy is reassuring, and I might > be missing something. I’m also not sure how to deal with the backing strscpy: > weak symbol, or something else... At least there aren’t (yet) any > arch-specific implementations of strscpy to deal with, but obviously they’d > still need to be supportable. > > In my tests, this all gets optimised away, and we end up with code such as > > strscpy(raead.type, "aead", sizeof(raead.type)); > > being compiled down to > > movl $1684104545, 4(%rsp) > > on x86-64, and non-constant code being compiled down to a direct __strscpy > call. Thanks for the details! Yeah, that seems nice. I wonder if there is a sensible way to combine these also with the stracpy*() proposal[1], so the call in your example above could just be: stracpy(raead.type, "aead"); (It seems both proposals together would have the correct result...) [1] https://lkml.kernel.org/r/201907221031.8B87A9DE@keescook -- Kees Cook