Re: [PATCH 8/9] x86/dumpstack: Save first regs set for the executive summary

[Linus Torvalds]

On Fri, Mar 16, 2018 at 4:48 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Thu, Mar 15, 2018 at 02:01:32PM -0500, Josh Poimboeuf wrote:
>> no_context() has the following line, right before it calls oops_end():
>>
>>       /* Executive summary in case the body of the oops scrolled away */
>>       printk(KERN_DEFAULT "CR2: %016lx\n", address);
>>
>> I think that line can now be removed, since the executive summary
>> __show_regs() will include CR2.
>
> Good idea. Done.

NOOOO!

Guys, %cr2 CAN AND DOES CHANGE!

The reason we do that

        printk(KERN_DEFAULT "CR2: %016lx\n", address);

is because WE ARE NOT PRINTING OUT THE CURRENT CR2 REGISTER!

This is really damn important.

The "address" register contains the CR2 value as it was read *very*
early in the page fault case, before we enabled interrupts, and before
we did various random things that can cause further page faults and
change CR2!

So the executive summary that does __show_regs() may end up showing
something completely different than the actual faulting address,
because we might have taken a vmalloc-space exception in the meantime,
for example.

Do *NOT* get rid of that thing.

You're better off getting rid of the CR2 line from __show_regs(),
because it can be dangerously confusing. It's not actually part of the
saved register state at all, it's something entirely different. It's
like showing the current eflags rather than the eflags saved on the
faulting stack.

                     Linus