Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAHk-=wjc-ktbOr7ZHMY8gfAmHxUK+aMdDsQjeh+BvmQwnQfN_g@mail.gmail.com>
 <20200701184131.GI2786714@ZenIV.linux.org.uk> <CAHk-=wj_2v9m+yZioE4vOLGW1mc9SBa5+++LdeJ86aEeB5OXcw@mail.gmail.com>
 <20200701195914.GK2786714@ZenIV.linux.org.uk> <CAHk-=wj-CYhKZR8ZKQgi=VTx=o7n6dtwPXikvgkJ3SdiqRPd8A@mail.gmail.com>
 <87lfk26nx4.fsf@mpe.ellerman.id.au> <CAHk-=wicOPQwuDUzFyDTBgr4UvQJHPdCX7_6BLaK6cve6CqBSg@mail.gmail.com>
 <20200702201755.GO2786714@ZenIV.linux.org.uk>
In-Reply-To: <20200702201755.GO2786714@ZenIV.linux.org.uk>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 2 Jul 2020 13:32:34 -0700
Message-ID: <CAHk-=whW7qYEK-MJMS9gKf-K4cBNGTq0pv-5wo4bqE_QtUfkDw@mail.gmail.com>
Subject: Re: objtool clac/stac handling change..
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     Michael Ellerman <mpe@ellerman.id.au>,
        Christophe Leroy <christophe.leroy@c-s.fr>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jul 2, 2020 at 1:18 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>         stac();
>         XSTATE_OP(XSAVE, buf, -1, -1, err);
>         clac();
>
> Rely upon objtool not noticing that we have, in effect, clac() in a state
> where AC is already cleared?  We could massage that thing to take a label,
> but it wouldn't be pretty...

Ugh, the above is bad anyway.

It doesn't use _ASM_EXTABLE_UA, so it won't warn about the noncanonical cases.

Yeah, it would need to be turned into a "jump out" instead of just "jump over".

Which it damn well should do anyway.,

That code should be taken behind a shed and shot. It does so many
things wrong that it's not even funny. It shouldn't do stac/clac on
its own.

At least it could use the "user_insn()" helper, which does it inside
the asm itself, has the right might_fault() marking (but not the
address check), and which can be trivially changed to have the fixup
jump be to after the "ASM_CLAC".

Or maybe it could use "asm goto" for that exception handling, because
it doesn't have any outputs (except for the error, which is exactly
what the goto case is for).

But no, at no point should we "rely on objtool not noticing". In fact,
I think Josh's patch would have made objtool notice, and it would have
been a good thing, because that code is unbelievably ugly and needs to
be cleaned up anyway.

Am I surprised that we have hacky stuff in our magic FPU/MMX/AVX state
handling? No I'm not.

I'm getting to the point where I'm thinking maybe all the horrendous
mis-steps from Intel over the last years are a good thing, and ARM
will take over, and we won't have to deal with this a decade from now.

                      Linus