Hi Linus,
On Sat, Aug 17, 2024 at 09:26:21AM GMT, Linus Torvalds wrote:
> On Sat, 17 Aug 2024 at 01:48, Alejandro Colomar <alx@xxxxxxxxxx> wrote:
> >
> > I would compact the above to:
> >
> > len = strlen(s);
> > buf = kmalloc_track_caller(len + 1, gfp);
> > if (buf)
> > strcpy(mempcpy(buf, s, len), "");
>
> No, we're not doing this kind of horror.
Ok.
> If _FORTIFY_SOURCE has problems with a simple "memcpy and add NUL",
> then _FORTIFY_SOURCE needs to be fixed.
_FORTIFY_SOURCE works (AFAIK) by replacing the usual string calls by
oneis that do some extra work to learn the real size of the buffers.
This means that for _FORTIFY_SOURCE to work, you need to actually call a
function. Since the "add NUL" is not done in a function call, it's
unprotected (except that sanitizers may protect it via other means).
Here's the fortified version of strcpy(3) in the kernel:
$ grepc -h -B15 strcpy ./include/linux/fortify-string.h
/**
* strcpy - Copy a string into another string buffer
*
* @p: pointer to destination of copy
* @q: pointer to NUL-terminated source string to copy
*
* Do not use this function. While FORTIFY_SOURCE tries to avoid
* overflows, this is only possible when the sizes of @q and @p are
* known to the compiler. Prefer strscpy(), though note its different
* return values for detecting truncation.
*
* Returns @p.
*
*/
/* Defined after fortified strlen to reuse it. */
__FORTIFY_INLINE __diagnose_as(__builtin_strcpy, 1, 2)
char *strcpy(char * const POS p, const char * const POS q)
{
const size_t p_size = __member_size(p);
const size_t q_size = __member_size(q);
size_t size;
/* If neither buffer size is known, immediately give up. */
if (__builtin_constant_p(p_size) &&
__builtin_constant_p(q_size) &&
p_size == SIZE_MAX && q_size == SIZE_MAX)
return __underlying_strcpy(p, q);
size = strlen(q) + 1;
/* Compile-time check for const size overflow. */
if (__compiletime_lessthan(p_size, size))
__write_overflow();
/* Run-time check for dynamic size overflow. */
if (p_size < size)
fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE, p_size, size, p);
__underlying_memcpy(p, q, size);
return p;
}
> We don't replace a "buf[len] = 0" with strcpy(,""). Yes, compilers may
> simplify it, but dammit, it's an unreadable incomprehensible mess to
> humans, and humans still matter a LOT more.
I understand. While I don't consider it unreadable anymore (I guess I
got used to it), it felt strange at first.
>
> Linus
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
Attachment:
signature.asc
Description: PGP signature
