Re: git: cf8e5289a110 - main - include: ssp: round out fortification of current set of headers

On 11/11/24 15:13, John Baldwin wrote:
> On 7/13/24 22:23, Kyle Evans wrote:
>> The branch main has been updated by kevans:
>>
>> URL: 
>> https://cgit.FreeBSD.org/src/commit/?id=cf8e5289a110954600f135024d1515a77d0ae34d
>>
>> commit cf8e5289a110954600f135024d1515a77d0ae34d
>> Author:     Kyle Evans <kevans@FreeBSD.org>
>> AuthorDate: 2024-07-13 05:16:10 +0000
>> Commit:     Kyle Evans <kevans@FreeBSD.org>
>> CommitDate: 2024-07-13 05:16:24 +0000
>>
>>      include: ssp: round out fortification of current set of headers
>>      ssp/ssp.h needed some improvements:
>>       - `len` isn't always a size_t, it may need casted
>>       - In some cases we may want to use a len that isn't specified as a
>>          parameter (e.g., L_ctermid), so __ssp_redirect() should be more
>>          flexible.
>>       - In other cases we may want additional checking, so pull all of 
>> the
>>          declaration bits out of __ssp_redirect_raw() so that some 
>> functions
>>          can implement the body themselves.
>>      strlcat/strlcpy should be the last of the fortified functions 
>> that get
>>      their own __*_chk symbols, and these cases are only done to be
>>      consistent with the rest of the str*() set.
>>      Reviewed by:    markj
>>      Sponsored by:   Klara, Inc.
>>      Sponsored by:   Stormshield
>>      Differential Revision:  https://reviews.freebsd.org/D45679
> 
> For the change in <sys/libkern.h>, is the intention for <ssp/ssp.h> to only
> be included in userspace binaries that use this header for some reason?  As
> it is, there are a handful of files compiled in the kernel that use remove
> -nostdinc from CFLAGS to access intrinsic headers for things like crypto
> instructions and those files end up including all of <ssp/ssp.h> in the
> kernel, e.g. this from armv8crypto:
> 
> # Remove -nostdinc so we can get the intrinsics.
> armv8_crypto_wrap.o: armv8_crypto_wrap.c
>      ${CC} -c ${CFLAGS:C/^-O2$/-O3/:N-nostdinc:N-mgeneral-regs-only} \
>          -I${SRCTOP}/sys/crypto/armv8 \
>          ${WERROR} ${PROF} \
>           -march=armv8-a+crypto ${.IMPSRC}
>      ${CTFCONVERT_CMD}
> 
> For CHERI this breaks in an obscure way (which is why I discovered this),
> but I'm curious what the intention is?  Should the kernel always be
> using the fallback definition of __ssp_real?
> 

I think this was meant to address some cross-build issue somewhere in 
our bootstrapped stuff; if it was a kernel vs. userspace thing I don't 
see why I wouldn't have used _KERNEL instead.  The kernel should always 
use the stub that just defines it back to the name passed in, though, so 
we could definitely add a '&& !defined(_KERNEL)' there.

Thanks,

Kyle Evans