[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=wgrT9++rFTnM1vh3bwx2Pcc18anDGQCwEL+0d2nDm3p+A@mail.gmail.com>
Date: Mon, 28 Apr 2025 12:34:27 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Bill Wendling <morbo@...gle.com>
Cc: Nathan Chancellor <nathan@...nel.org>, Josh Poimboeuf <jpoimboe@...nel.org>,
Harry Wentland <harry.wentland@....com>, Leo Li <sunpeng.li@....com>,
Alex Deucher <alexander.deucher@....com>, Christian König <christian.koenig@....com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Justin Stitt <justinstitt@...gle.com>,
"the arch/x86 maintainers" <x86@...nel.org>, dri-devel <dri-devel@...ts.freedesktop.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, llvm@...ts.linux.dev
Subject: Re: clang and drm issue: objtool warnings from clang build
On Mon, 28 Apr 2025 at 11:08, Bill Wendling <morbo@...gle.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.
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.
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
Powered by blists - more mailing lists