Re: [PATCH v4 3/5] x86: extend do_mmu_update() to support returning the old PTE value

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/10/2026 5:48 pm, Kevin Lampis wrote:
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 609c8595a487..ab09e0703da4 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4212,37 +4213,80 @@ long do_mmu_update(
>                       update_flags = PTE_UPDATE_PRESERVE_AD;
>                  else if ( cmd == MMU_PT_UPDATE_NO_TRANSLATE )
>                       update_flags = PTE_UPDATE_NO_TRANSLATE;
> +                else if ( cmd == MMU_PT_UPDATE_SWAP )
> +                     update_flags = PTE_UPDATE_SWAP;
>  
>                  switch ( page->u.inuse.type_info & PGT_type_mask )
>                  {
>                  case PGT_l1_page_table:

{

> +                    l1_pgentry_t ol1e = {};
>                      rc = mod_l1_entry(va, l1e_from_intpte(req.val), mfn,
> -                                      update_flags, v, pg_owner, NULL);
> +                                      update_flags, v, pg_owner, &ol1e);
> +
> +                    if ( !rc && cmd == MMU_PT_UPDATE_SWAP )
> +                    {
> +                        req.val = ol1e.l1;
> +                        if ( unlikely(copy_to_guest(ureqs, &req, 1)) )
> +                            rc = -EFAULT;
> +                    }

}

You need braces, otherwise:

arch/x86/mm.c:4227:21: error: label followed by a declaration is a C23 
extension [-Werror,-Wc23-extensions]
 4227 |                     l1_pgentry_t ol1e = {};
      |                     ^


It's also a MISRA violation.

https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/3046070649

>                      break;
>  
>                  case PGT_l2_page_table:
> +                    l2_pgentry_t ol2e = {};
> +
>                      if ( unlikely(pg_owner != pt_owner) )
>                          break;
> +
>                      rc = mod_l2_entry(va, l2e_from_intpte(req.val), mfn,
> -                                      update_flags, v, NULL);
> +                                      update_flags, v, &ol2e);
> +
> +                    if ( !rc && cmd == MMU_PT_UPDATE_SWAP )
> +                    {
> +                        req.val = ol2e.l2;
> +                        if ( unlikely(copy_to_guest(ureqs, &req, 1)) )
> +                            rc = -EFAULT;
> +                    }
> +
>                      if ( !rc )
>                          flush_linear_pt = true;

You must set flush_linear_pt based on the success of mod_l2_entry()
before potentially clobbering rc with -EFAULT.

As this logic stands, it's an easy privilege escalation into Xen, by
causing Xen to skip a safety TLB flush.


Other than these two, everything looks in order.

~Andrew