On Wed, Jul 03, 2019 at 04:29:42PM -0400, Steven Rostedt wrote: > 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?
So I don't particularly care about stable; and the band-aids (trace_irqs_off_cr2) is known broken so I really don't see the point. That said, these patches should apply to most recent kernels (post PTI) without too much rejects.
