On 3/27/23 13:14, David Woodhouse wrote:
On Mon, 2023-03-27 at 19:47 +0200, Borislav Petkov wrote:
Making sure that the stack protector is either disabled or properly
set up, and disabling any instrumentation/profiling/debug crap that
isn't initialized yet.
Lemme dump brain of what Tom and I were talking about today so that it
is documented somewhere.
* re: stack protector: I was thinking to mark this function
__attribute__((no_stack_protector))
but gcc added the function attribute way later:
~/src/gcc/gcc.git> git tag --contains 346b302d09c1e6db56d9fe69048acb32fbb97845
basepoints/gcc-12
basepoints/gcc-13
releases/gcc-11.1.0
releases/gcc-11.2.0
releases/gcc-11.3.0
releases/gcc-12.1.0
releases/gcc-12.2.0
which means, that function would have to live somewhere in a file which
has stack protector disabled. One possible place would be
arch/x86/mm/mem_encrypt_identity.c which is kinda related.
Shouldn't the rest of head64.c have the stack protector disabled, for
similar reasons?
* re: stack: in order to be able to call a C function that early, we'd
have to put the VA of the initial stack back into %rsp as we switch
pagetables a bit earlier in there (thx Tom).
Hm, don't you have a stack at the point you added that call? I thought
you did? It doesn't have to be *the* stack for the AP in question.
Just "a" stack. And you have the lock on the real-mode one that you're
using.
Unfortunately RSP has the identity mapped stack value and when the
pagetable switch is performed the mapping to that stack is lost. It would
need to be updated to the equivalent of __va(RSP) to get a stack that can
be used without page faulting.
Thanks,
Tom
So by then, doing all that cargo-cult just in order to not have a bunch
of lines in asm doesn't sound all that great anymore.
* The __head per-function attribute is easily solved by lifting the
__head define into a common header.
So meh, dunno. I guess we can do the asm thing for now, until a cleaner
solution without too many warts presents itself.
Hm, doesn't most of that just go away (or at least become "Already
Broken; Someone Else's Problem™") if you just concede to put your new C
function into head64.c along with a whole bunch of other existing
CONFIG_AMD_MEM_ENCRYPT support?
(We still have to fix it if it's Someone Else's Problem, of course.
It's just that you don't have to count that complexity towards your own
part.)
