Re: [PATCH v2 for-4.14] x86/livepatch: Make livepatching compatible with CET Shadow Stacks

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 2020-06-26 13:24, Andrew Cooper wrote:
> Just like the alternatives infrastructure, the livepatch infrastructure
> disables CR0.WP to perform patching, which is not permitted with CET active.
> 
> Modify arch_livepatch_{quiesce,revive}() to disable CET before disabling WP,
> and reset the dirty bits on all virtual regions before re-enabling CET.
> 
> One complication is that arch_livepatch_revive() has to fix up the top of the
> shadow stack.  This depends on the functions not being inlined, even under
> LTO.  Another limitation is that reset_virtual_region_perms() may shatter the
> final superpage of .text depending on alignment.
> 
> This logic, and its downsides, are temporary until the patching infrastructure
> can be adjusted to not use CR0.WP.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> CC: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
> CC: Paul Durrant <paul@xxxxxxx>
> 
> For 4.14.  This is a bug in a 4.14 feature, with a very low risk to non-CET
> usecases.
> 
> v2:
>  * nolinline, and extra ifdefary
>  * Expand comments
> ---
>  xen/arch/x86/livepatch.c         | 35 +++++++++++++++++++++++++++++++++--
>  xen/common/virtual_region.c      | 15 +++++++++++++++
>  xen/include/xen/virtual_region.h |  1 +
>  3 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
> index 901fad96bf..49f0d902e5 100644
> --- a/xen/arch/x86/livepatch.c
> +++ b/xen/arch/x86/livepatch.c
> @@ -12,6 +12,7 @@
>  #include <xen/livepatch.h>
>  #include <xen/sched.h>
>  #include <xen/vm_event.h>
> +#include <xen/virtual_region.h>
>  
>  #include <asm/fixmap.h>
>  #include <asm/nmi.h>
> @@ -56,18 +57,48 @@ int arch_livepatch_safety_check(void)
>      return -EBUSY;
>  }
>  
> -int arch_livepatch_quiesce(void)
> +int noinline arch_livepatch_quiesce(void)
>  {
> +    /* If Shadow Stacks are in use, disable CR4.CET so we can modify CR0.WP. 
> */
> +    if ( cpu_has_xen_shstk )

Should this be:
    if ( IS_ENABLED(CONFIG_XEN_SHSTK) && cpu_has_xen_shstk )

to match arch_livepatch_revive?

Ross