Re: [Xen-devel] [PATCH] x86/entry: Fix passing 6th argument for compat hypercalls

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/03/18 19:55, Jason Andryuk wrote:
> Commit ec05090403ef4d760fbe701e31afd0f0edc414d5 ("x86/entry: Erase guest
> GPR state on entry to Xen") zero-ed %rbp, compat arg 6, but it is not
> restored before passing to hypercalls.  We need to pass the saved compat
> arg 6 to the hypercall in r9, the 6th function argument.
>
> Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> ---

Jan: for reference, this is precisely the reason why I recommended that
you backport the minimal 51e5d6c7a29 which I provided, rather than try
to fix up the existing on-stack logic.

Looking at that commit, there is another bug in LOAD_C_CLOBBERED, where
passing compat=0 xc=0 corrupts %rcx with whatever was on the stack for %r10

> This is against Xen 4.6.  I believe it is also applicable to 4.7.
>
>  xen/arch/x86/x86_64/compat/entry.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/x86_64/compat/entry.S 
> b/xen/arch/x86/x86_64/compat/entry.S
> index bc1f509672..b99b142e45 100644
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -56,7 +56,7 @@ UNLIKELY_END(msi_check)
>          xchgl %ecx,%esi              /* Arg 2, Arg 4 */
>          movl  %edx,%edx              /* Arg 3        */
>          movl  %edi,%r8d              /* Arg 5        */
> -        movl  %ebp,%r9d              /* Arg 6        */
> +        movl  UREGS_rbp(%rsp),%r9d   /* Arg 6        */
>          movl  UREGS_rbx(%rsp),%edi   /* Arg 1        */

This looks correct, (but there is a good reason why I purged this code
from the codebase 3 releases ago).

~Andrew

>  #define SHADOW_BYTES 0  /* No on-stack shadow state */
>  #endif


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel