On 11/11/24 13:43, Kyle Evans wrote:
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 <[email protected]>
AuthorDate: 2024-07-13 05:16:10 +0000
Commit: Kyle Evans <[email protected]>
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.
Ok, thanks, that's what I've done locally. I'll throw it into a review in
a bit.
--
John Baldwin
