On Wed, 2022-12-21 at 11:41 +0100, Borislav Petkov wrote: > On Wed, Dec 21, 2022 at 12:37:51AM +0000, Edgecombe, Rick P wrote: > > You mean having separate paths for kernel IBT and user shadow stack > > that compile out? I guess it could just all be in place if > > CONFIG_X86_CET is in place. > > > > I don't know, I thought it was relatively clean, but I can remove > > it. > > Yeah, I'm wondering if we really need the ifdeffery. I always > question > ifdeffery because it is a) ugly, b) a mess to deal with and having it > is > not really worth it. Yeah, we save a couple of KBs, big deal. > > What would practically happen is, shadow stack will be default- > enabled > on the majority of kernels out there - distro ones - so it will be > enabled practically everywhere. > > And it'll be off only in some self-built kernels which are the very > small minority. > > And how much are the space savings with the whole set applied, with > and > without the Kconfig item enabled? Probably only a couple of KBs. Oh, you mean the whole Kconfig thing. Yea, I mean I see the point about typical configs. But at least CONFIG_X86_CET seems consistent with CONFIG_INTEL_TDX_GUEST, CONFIG_IOMMU_SVA, etc. What about moving it out of traps.c to a cet.c, like exc_vmm_communication for CONFIG_AMD_MEM_ENCRT? Then the inclusion logic lives in the build files, instead of an ifdef. > > And if so, I'm thinking we could at least make the traps.c stuff > unconditional - it'll be there but won't run. Unless we get some > weird > #CP but it'll be caught by do_unexpected_cp(). > > And you have feature tests everywhere so it's not like it'll get > "misused". > > And when you do that, you'll have everything a lot simpler, a lot > less > Kconfig items to build-test and all good. > > Right? > > Or am I completely way off into the weeds here and am missing an > important aspect...? > One aspect that has come up a couple of times, is how closely related all these CET features are (or aren't). Shadow stack and IBT are mostly separate, but do share an xfeature and an exception type. Similarly for supervisor and user mode support for either of the CET features. So maybe that is what is unusual here. There are some aspects that make them look like separate features, which leads people to think they should be separate in the code. But actually separating them leads to excess ifdefery. To me, putting the whole handler in even if there are no CET features seems like too much. Leaving it in unconditionally is apparently also inconsistent with some of the previous traps.c decisions. So I'd leave CONFIG_X86_CET at least and it can help anyone building super stripped down kernels. But I'm ok with whatever people think.
