https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116457
--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> --- (In reply to Arsen Arsenović from comment #6) > (In reply to Andrew Pinski from comment #5) > > > but it is already in reliable real-world use. > > > > Then that real world use is broken. > > > > >It's a pretty isolated change and doesn't impact any later stages of > > >compilation. > > > > It is not exactly isolated, that is if there is an option, someone will try > > it and then get broken code. Did you run the full GCC testsuite with it > > turned on to see what breaks? Do you have a formal definition of what works > > with it and what does NOT? > That isn't really a useful measure - I suspect a lot of the testsuite would > break with -Ofast forced-on, for instance. I am more worried about getting it correct and warning/error message detection without that this feature should never go in because it is too easy to shoot yourself in the foot. On the -Ofast side of things, most of the ieee tests are limited to the ieee directory on purpose since there are some floating point representations that don't have NaNs/Infs (or denormals flush to zero); so I doubt there will be much fall out even from -Ofast due to the tests happening in the past on those targets (spu float for an example which was supported until I think GCC 6). > > WRT possible user error, one needs to opt-in for this (via an attribute, but > the plugin this'd replace currently also automatically shuffles vtable-style > structs and I think that is a footgun but it is what it is) User error but allowing it is not a good idea. Again it should warn/error out and there needs to be a lot of tests added. Otherwise you will get the wrong answer even in the kernel sometimes and a security issue slipping in due to incorrect usage is 100% not something you want. > > > Why have a seperate option for the seed? Rather than reuse > > -frandomize-layout-seed ? Is it because you want to force folks to always > > use a different seed from the seed which is going to be there for > > reproducibility or just because it is another obscure option? > > I see at least one reason: not having to interact with Kconfig more than > absolutely necessary. That is not answer to the question. Just what the Linux kernel does. > > > This could introduce new security issues just by having it, rather than > > security by obscurity. For an example someone's code is not ready for this > > layout change but then it messes up. > > > > Also why only C and not C++? At least in C++ you have local linkage structs > > (e.g. structs defined inside an anonymous namespaces) so you don't have a > > push to force things to be consistent between TUs. > this wouldn't be the first kernel-induced C-only feature, really And there was a movement to use C++ in the kernel. Is there some mechanism in the design to make sure this does not leave the TU? Because it is not obvious from any of the designs currently nor from clang implementation (which is missing a lot of the warning/error part to make sure it would still stay valid). > > > Does clang (or the plugin) warn if a struct with randomize_layout is used > > with an exported symbol? what about taking the address and then passing it > > around? Do es it have some kind of flow sensitive warning because without > > that it is 100% a security issue waiting to happen. > no, the plugin is extremely primitive. due to the seed being constant > across a kernel build, there isn't really a problem wrt linking TUs together. what happens if you have 2 structs marked as randomize_layout but the order of the definition is different between 2 TUs? Then this breaks unless you only have them not going outside the TU. If you allow outside of the TU usage, then that is 100% a security issue because then you still need to write out somewhere the order and expose that to maybe even modules. This is why I said this is not a good idea at all unless you can prove it does not go outside of the TU and from what I understand that is not useful for the Linux kernel folks, they need something that works across TUs.
