https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114759
--- Comment #3 from Peter Bergner <bergner at gcc dot gnu.org> --- (In reply to Segher Boessenkool from comment #2) >> 1. We always define the __ROP_PROTECT__ predefined macro [snip] > > No. Whenever the -mrop-protect option is in effect, we should do that > predefine. What we do now is: gcc -dM -E -mcpu=power10 test.c | grep ROP gcc -dM -E -mcpu=power10 -mrop-protect test.c | grep ROP #define __ROP_PROTECT__ 1 gcc -dM -E -mcpu=power4 -mrop-protect test.c | grep ROP #define __ROP_PROTECT__ 1 ...and that last compile is a wrong code bug. If we want to continue to silently disable ROP protection with -mcpu=power4, then we also need to not define __ROP_PROTECT__. If we decide we want an error for this, which I mentioned later as an solution, then this is fixed automatically by doing that. > If you want to refuse the option without a -mcpu= that can generate useful > code for it, that's fine, but that is not what we do. Instead, we generate > code that will do the ROP-protection boogaloo on CPUs that implement support > for that, and does nothing otherwise. We do not currently do "nothing" when we see -mcpu=power4 -mrop-protect. Yes, we do not emit the hashst and hashchk insns, but we *do* emit the __ROP_PROTECT__ predefined macro and that is bad. The most common usage of that macro is in .S assembler files and if we define __ROP_PROTECT__ in the wrong cases, they can end up with assembler errors. Again, if we decide to emit an error for -mcpu=power4 -mrop-protect, then this is just fixed automatically. >> 2. We always disable shrink-wrapping when -mrop-protect is used, [...] > > Yes, this is problematic, and seems to be completely unnecessary. For the case where we silently ignore -mcpu=power4 -mrop-protect, it is completely unnecessary. If we decide to emit an error for this instead, then like the above, this is just automatically fixed. > By exactly the same argument we should *also* do ROP-protection in all > leaf functions, btw! I'm not 100% convinced we need to "protect" leaf functions, since the return address of the leaf function ever makes it onto the stack to be potentially corrupted. Can you explain how a leaf-function could be attacked if we never save its return address to the stack? >> 3. We silently disable ROP protection for everything other than >> -mcpu=power10. The binutils assembler accepts the ROP insns back >> to Power8, so we should emit them for Power8 and later. > > The ISA claims it will work for anything after ISA 2.04, even. True, but given the binutils assembler doesn't accept hashst and hashchk for anything before Power8, it seemed convenient to match that behavior. If we enable it for ISA 2.04 and later, then we either have to fix binutils to do the same (which we can do), but we still run the risk of some compiles failing because the user is using an older unfixed assembler. >> 4. We give an error when -mrop-protect is used with any -mabi=ABI >> value not equal to ELFv2, [...] > > Yes, we should make it work everywhere. Even on -m32. But it requires > adjusting the ABI as well! That's a nice goal, but I'd like to fix the present issues before tackling expanding its use to other ABIs. So the first question to ask is, do we want to silently disable (maybe with a warning) emitting ROP instructions if used with -mcpu=CPU or -mabi=ABI that we don't want or can't emit them for? ...or do we want to produce an error? The answer to this question will help guide us on how to fix the other issues or whether we even have to do anything for them.
