On Wed, 3 Jul 2019 22:22:31 +0200 Peter Zijlstra <pet...@infradead.org> wrote:
> On Wed, Jul 03, 2019 at 12:27:34PM +0200, root wrote: > > Despire the current efforts to read CR2 before tracing happens there > > still exist a number of possible holes: > > > > idtentry page_fault do_page_fault has_error_code=1 > > call error_entry > > TRACE_IRQS_OFF > > call trace_hardirqs_off* > > #PF // modifies CR2 > > > > CALL_enter_from_user_mode > > __context_tracking_exit() > > trace_user_exit(0) > > #PF // modifies CR2 > > > > call do_page_fault > > address = read_cr2(); /* whoopsie */ > > > > And similar for i386. > > > > Fix it by pulling the CR2 read into the entry code, before any of that > > stuff gets a chance to run and ruin things. > > > > Ideally we'll clean up the entry code by moving this tracing and > > context tracking nonsense into C some day, but let's not delay fixing > > this longer. > > > > > @@ -1180,10 +1189,10 @@ idtentry xenint3 do_int3 > > has_error_co > > #endif > > > > idtentry general_protection do_general_protection has_error_code=1 > > -idtentry page_fault do_page_fault has_error_code=1 > > +idtentry page_fault do_page_fault > > has_error_code=1 read_cr2=1 > > > > #ifdef CONFIG_KVM_GUEST > > -idtentry async_page_fault do_async_page_fault has_error_code=1 > > +idtentry async_page_fault do_async_page_fault has_error_code=1 > > read_cr2=1 > > #endif > > While going over the various idt handlers, I found that we probably also > need read_cr2 on do_double_fault(), otherwise it is susceptible to the > same problem. > BTW, do you plan on making this for stable? Even though it's rather invasive. Or should we just apply the band-aids first, have them backported to stable, and then put this change on top of them for upstream? -- Steve
