On Mon, Aug 05, 2019 at 07:28:54PM +0200, Borislav Petkov wrote: > > 1: > > @@ -1571,7 +1572,8 @@ nested_nmi: > > pushq %rdx > > pushfq > > pushq $__KERNEL_CS > > - pushq $repeat_nmi > > + leaq repeat_nmi(%rip), %rdx > > + pushq %rdx > > > > /* Put stack back */ > > addq $(6*8), %rsp > > @@ -1610,7 +1612,11 @@ first_nmi: > > addq $8, (%rsp) /* Fix up RSP */ > > pushfq /* RFLAGS */ > > pushq $__KERNEL_CS /* CS */ > > - pushq $1f /* RIP */ > > + pushq $0 /* Future return address */ > > + pushq %rax /* Save RAX */ > > + leaq 1f(%rip), %rax /* RIP */ > > + movq %rax, 8(%rsp) /* Put 1f on return address */ > > + popq %rax /* Restore RAX */ > > Can't you just use a callee-clobbered reg here instead of preserving > %rax? As Peter stated later in this thread, we only have the IRQ stack frame saved here, because we just took an NMI, and this is the logic to determine if it was a nested NMI or not (where we have to be *very* careful about touching the stack!) That said, the code modified here is to test the NMI nesting logic (only enabled with CONFIG_DEBUG_ENTRY), and what it is doing is re-enabling NMIs before calling the first NMI handler, to help trigger nested NMIs without the need of a break point or page fault (iret enables NMIs again). This code is in the path of the "first nmi" (we confirmed that this is not nested), which means that it should be safe to push onto the stack. Yes, we need to save and restore whatever reg we used. The only comment I would make is to use %rdx instead of %rax as that has been our "scratch" register used before saving pt_regs. Just to be consistent. -- Steve
