Re: [PATCH v3] x86/traps: Re-enable interrupts after reading cr2 in the #PF handler

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

On Fri Sep 27, 2024 at 3:23 PM BST, Alejandro Vallejo wrote:
> Hitting a page fault clobbers %cr2, so if a page fault is handled while
> handling a previous page fault then %cr2 will hold the address of the
> latter fault rather than the former. This patch makes the page fault
> path delay re-enabling IRQs until %cr2 has been read in order to ensure
> it stays consistent.
>
> A similar argument holds in additional cases, but they happen to be safe:
>
>   * %dr6 inside #DB: Safe because IST exceptions don't re-enable IRQs.
>   * MSR_XFD_ERR inside #NM: Safe because AMX isn't used in #NM handler.
>
> While in the area, remove redundant q suffix to a movq in entry.S and
> add space after the comma.
>
> Fixes: a4cd20a19073 ("[XEN] 'd' key dumps both host and guest state.")
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> v3:
>   * s/dispatch_handlers/dispatch_exceptions/
>   * Updated commit message, spelling out the state of #DB and #NM, and
>     state an existing race with debug keys.

Bah, I didn't refresh the patch with the latest commit message. It was meant to
be:

  x86/traps: Re-enable interrupts after reading cr2 in the #PF handler

  Hitting a page fault clobbers %cr2, so if a page fault is handled while
  handling a previous page fault then %cr2 will hold the address of the latter
  fault rather than the former. In particular, if a debug key handler happens
  to trigger during #PF and before %cr2 is read, and that handler itself
  encounters a #PF, then %cr2 will be corrupt for the outer #PF handler.

  This patch makes the page fault path delay re-enabling IRQs until %cr2 has
  been read in order to ensure it stays consistent.

  A similar argument holds in additional cases, but they happen to be safe:

    * %dr6 inside #DB: Safe because IST exceptions don't re-enable IRQs.
    * MSR_XFD_ERR inside #NM: Safe because AMX isn't used in #NM handler.

  While in the area, remove redundant q suffix to a movq in entry.S and
  add space after the comma.

  Fixes: a4cd20a19073 ("[XEN] 'd' key dumps both host and guest state.")
  Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
  Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Would whoever planned to commit this mind replacing the commit msg on commit?
Otherwise I'll just resend.

Cheers,
Alejandro