On Fri, 2020-05-15 at 16:56 -0700, Dave Hansen wrote: > On 5/15/20 4:29 PM, Yu-cheng Yu wrote: > > On Fri, 2020-05-15 at 15:43 -0700, Dave Hansen wrote: > > > Basically, if there ends up being a bug in an app that violates the > > > shadow stack rules, the app is broken, period. The only recourse is to > > > have the kernel disable CET and reboot. > > > > > > Is that right? > > > > You must be talking about init or any of the system daemons, right? > > Assuming we let the app itself start CET with an arch_prctl(), why would that be > > different from the current approach? > > You're getting ahead of me a bit here. > > I'm actually not asking directly about the prctls() or advocating for a > different approach. The MPX approach of _requiring the app to make a > prctl() was actually pretty nasty because sometimes threads got created > before the prctl() could get called. Apps ended up inadvertently > half-MPX-enabled. Not fun. > > Let's say we have an app doing silly things like retpolines. (Lots of > app do lots of silly things). It gets compiled in a distro but never > runs on a system with CET. The app gets run for the first time on a > system with CET. App goes boom. Not init, just some random app, say > /usr/bin/ldapsearch. > > What's my recourse as an end user? I want to run my app and turn off > CET for that app. How can I do that? GLIBC_TUNABLES=glibc.tune.hwcaps=-SHSTK,-IBT > > > > > Can a binary compiled without CET run CET-enabled code? > > > > > > > > Partially yes, but in reality somewhat difficult. > > > ... > > > > - If a not-CET application does fork(), and the child wants to turn on CET, it > > > > would be difficult to manage the stack frames, unless the child knows what is is > > > > doing. > > > > > > It might be hard to do, but it is possible with the patches you posted? > > > > It is possible to add an arch_prctl() to turn on CET. That is simple from the > > kernel's perspective, but difficult for the application. Once the app enables > > shadow stack, it has to take care not to return beyond the function call layers > > before that point. It can no longer do longjmp or ucontext swaps to anything > > before that point. It will also be complicated if the app enables shadow stack > > in a signal handler. > > Yu-cheng, I'm having a very hard time getting direct answers to my > questions. Could you endeavor to give succinct, direct answers? If you > want to give a longer, conditioned answer, that's great. But, I'd > appreciate if you could please focus first on clearly answering the > questions that I'm asking. > > Let me try again: > > Is it possible with the patches in this series to run a single- > threaded binary which was has GNU_PROPERTY_X86_FEATURE_1_SHSTK > unset to run with shadow stack protection? > > I think the answer is an unambiguous: "No". But I'd like to hear it > from you. No! > > > I think you're saying that the CET-enabled binary would do > > > arch_setup_elf_property() when it was first exec()'d. Later, it could > > > use the new prctl(ARCH_X86_CET_DISABLE) to disable its shadow stack, > > > then fork() and the child would not be using CET. Right? > > > > > > What is ARCH_X86_CET_DISABLE used for, anyway? > > > > Both the parent and the child can do ARCH_X86_CET_DISABLE, if CET is > > not locked. > > Could you please describe a real-world example of why > ARCH_X86_CET_DISABLE exists? What kinds of apps will use it, or *are* > using it? Why was it created in the first place? Currently, ld-linux turns off CET if the binary being loaded does not support CET. > > > > The JIT examples I mentioned previously run with CET enabled from the > > > > beginning. Do you have a reason to do this? In other words, if the JIT code > > > > needs CET, the app could have started with CET in the first place. > > > > > > Let's say I have a JIT'd sandbox. I want the sandbox to be > > > CET-protected, but the JIT engine itself not to be. > > > > I do not have any objections to this use case, but it needs some cautions as > > stated above. It will be much easier and cleaner if the sandbox is in a > > separate exec'ed task with CET on. > > OK, great suggestion! Could you do some research and look at the > various sandboxing techniques? Is imposing this requirement for a > separate exec'd task reasonable? Does it fit nicely with their existing > models? How about the Chrome browser and Firefox sandboxs? I will check. > > > > > Does this *code* work? Could you please indicate which JITs have been > > > > > enabled to use the code in this series? How much of the new ABI is in use? > > > > > > > > JIT does not necessarily use all of the ABI. The JIT changes mainly fix stack > > > > frames and insert ENDBRs. I do not work on JIT. What I found is LLVM JIT fixes > > > > are tested and in the master branch. Sljit fixes are in the release. > > > > > > Huh, so who is using the new prctl() ABIs? > > > > Any code can use the ABI, but JIT code CET-enabling part mostly do not use these > > new prctl()'s, except, probably to get CET status. > > Which applications specifically are going to use the new prctl()s which > this series adds? How are they going to use them? > > "Any code can use them" is not a specific enough answer. We have four arch_ptctl() calls. ARCH_X86_CET_DISABLE and ARCH_X86_CET_LOCK are used by ld-linux. ARCH_X86_CET_STATUS are used in many places to determine if CET is on. ARCH_X86_CET_ALLOC_SHSTK is used in ucontext related handling, but it can be use by any application to switch shadow stacks. > > > > > Where are the selftests/ for this new ABI? Were you planning on > > > > > submitting any with this series? > > > > > > > > The ABI is more related to the application side, and therefore most suitable for > > > > GLIBC unit tests. > > > > > > I was mostly concerned with the kernel selftests. The things in > > > tools/testing/selftests/x86 in the kernel tree. > > > > I have run them with CET enabled. All of them pass, except for the following: > > Sigreturn from 64-bit to 32-bit fails, because shadow stack is at a 64-bit > > address. This is understandable. > > That is not what I meant. I'm going to be as explicit: > > I expect you to create a test case which you will submit with these > patches and the test case will go into the tools/testing/selftests/x86 > directory in the kernel tree. This test case will exercise the kernel > functionality added in this series, especially the new prctl()s. I will submit the test case as a separate patch in response to this discussion, and combine with the series when the discussion concludes. > One a separate topic: You ran the selftests and one failed. This is a > *MASSIVE* warning sign. It should minimally be described in your cover > letter, and accompanied by a fix to the test case. It is absolutely > unacceptable to introduce a kernel feature that causes a test to fail. > You must either fix your kernel feature or you fix the test. > > This code can not be accepted until this selftests issue is rectified. Sure, I will do that. > > > > > The more complicated areas such as pthreads, signals, ucontext, > > > > fork() are all included there. I have been constantly running these > > > > tests without any problems. I can provide more details if testing is > > > > the concern. > > > > > > For something this complicated, with new kernel ABIs, we need an > > > in-kernel sefltest. > > > > > > MPX was not that much different from this feature. It required a > > > boatload of compiler and linker changes to function. Yet, there was a > > > simple in-kernel test for it that didn't require *any* of that big pile > > > of toolchain bits. > > > > > > Is there a reason we don't have one of those for CET? > > > > I have a quick test that checks shadow stack and ibt in both main program and in > > signals. Currently it is public on Github. If that is desired, I can submit it > > to the mailing list. > > Yes, that is desired. It must accompany this submission. It must also > exercise all of the new ABIs. Ok. Yu-cheng
