On Tue, Dec 05, 2023 at 04:01:50PM +0000, Edgecombe, Rick P wrote: > Hmm, I didn't realize you were planning to have the kernel support > upstream before the libc support was in testable shape. It's not a "could someone run it" thing - it's about trying ensure that we get coverage from people who are just running the selftests as part of general testing coverage rather than with the specific goal of testing this one feature. Even when things start to land there will be a considerable delay before they filter out so that all the enablement is in CI systems off the shelf and it'd be good to have coverage in that interval. > > What's the issue with working around the missing support? My > > understanding was that there should be no ill effects from repeated > > attempts to enable. We could add a check for things already being > > enabled > Normally the loader enables shadow stack and glibc then knows to do > things in special ways when it is successful. If it instead manually > enables in the app: > - The app can't return from main() without disabling shadow stack > beforehand. Luckily this test directly calls exit() > - The app can't do longjmp() > - The app can't do ucontext stuff > - The enabling code needs to be carefully crafted (the inline problem > you hit) > I guess it's not a huge list, and mostly tests will run ok. But it > doesn't seem right to add somewhat hacky shadow stack crud into generic > tests. Right, it's a small and fairly easily auditable list - it's more about the app than the double enable which was what I thought your concern was. It's a bit annoying definitely and not something we want to do in general but for something like this where we're adding specific coverage for API extensions for the feature it seems like a reasonable tradeoff. If the x86 toolchain/libc support is widely enough deployed (or you just don't mind any missing coverage) we could use the toolchain support there and only have the manual enable for arm64, it'd be inconsistent but not wildly so. > So you were planning to enable GCS in this test manually as well? How > many tests were you planning to add it like this? Yes, the current version of the arm64 series has the equivalent support for GCS. I was only planning to do this along with adding specific coverage for shadow stacks/GCS, general stuff that doesn't have any specific support can get covered as part of system testing with the toolchain and libc support. The only case beyond that I've done is some arm64 specific stress tests which are written as standalone assembler programs, those wouldn't get enabled by the toolchain anyway and have some chance of catching context switch or signal handling issues should they occur. It seemed worth it for the few lines of assembly it takes.
Attachment:
signature.asc
Description: PGP signature
