On Mon, Apr 28, 2025 at 12:34 PM Linus Torvalds <torva...@linux-foundation.org> wrote: > > On Mon, 28 Apr 2025 at 11:08, Bill Wendling <mo...@google.com> wrote: > > > > This situation is one of the > > easier ones: "do something other than fall into the next function"; > > Note that the "fall into the next function" is just something that > objtool notices. It *could* be "fall into the next basic block of the > same function, and objtool wouldn't warn, because objtool generally > wouldn't notice (there could be other things that make objtool notice, > of course - things like stack updates being out of whack or similar). > > But I really wish that clang would look at a "don't depend on UD as a > code generation model AT ALL" as a flag. > > The whole "this is undefined, so I'll generate something different" > model is just wrong. > > That said, there are certainly graduations of wrong: > > > but there are far more involved examples, of course. And even in this > > case, the compiler needs to know if a "trap" is okay, or would > > returning with garbage in %rax be okay. > > Honestly, the least wrong thing is to just NOT HAVE THE CHECK FOR ZERO AT ALL. > > IOW, just generate the divide instruction. > > I can almost guarantee that that will actually then generate the best > code too, because you'll probably just end up sharing the divide > instruction will all the *normal* cases. > I get what you're saying, I really do. I'm actually in the "playing Wack-A-Mole(tm) is far better than generating code that accidentally launches the nukes" crowd. The fact that the compiler silently generates something wrong is horrifying to me. The compiler has a ton of options to allow for "bad" math, but they're mostly (all?) for floating point operations. It has some for integers, like the -fwrapv you mentioned.
> So the best model is to literally remove that pointless and stupid "is > this a divide by zero" code. It's pointless and stupid because it > literally just makes for more work both for the compiler AND it > generates worse code. > > Why do extra work to generate worse code? > > Btu if some religious nutcase decides that "I will not generate divide > instructions if I know the divisor is zero" is a hill they will die > on, generating a "trap" instruction is certainly not inexcusable. > I'll see what I can do with this. I might be able to sneak a patch in past the religious nutcases. The fact that we have the two flags Nathan and I mentioned could indicate that someone will be amenable to the patch. -bw > Generating a random value for %eax is WRONG. Now, that said, it's > clearly less wrong than falling through to some unrelated code > entirely, so it would be an improvement on the *current* situation, > but that's like saying that getting shot in the foot is an improvement > on getting shot in the head: true, but if the alternative is not > getting shot at all, why is that "less bad" alternative even on the > table? > > The "just execute random code" is clearly so bad that it *should* be > off the table in the first place, and I don't understand why it is > what clang does now. It's just crazy. > > And yes, this really is a very potential and real security issue. In > the kernel I don't think we have this ever happening, partly because a > lot of configurations use gcc which afaik doesn't have this particular > horrendous model of UD. > > But this isn't just a kernel issue, it's a "anybody using clang to > build any program that might have security issues would be *insane* to > think this is a good model for dealing with UD". We do more checking > than most on the code generation, so we actually had tools that > noticed this odd code generaton. I can guarantee you that 99% of all > projects out there would never have even noticed. > > And who knows what cases we *don't* find. > > And obviously hopefully UD doesn't actually happen. But that's like > saying "hopefully we have no bugs". It's not reality. > > Using UD to change code generation really is a truly horrendously bad > idea in the first place, but doing it in anything where security might > matter takes "bad idea" to "just don't do this". > > Linus
