Re: [PATCH 4/5] x86_64: fix user trap during syscall with an invalid user stack

[Samuel Thibault]

Applied, thanks!

Luca Dariz, le lun. 27 févr. 2023 21:45:00 +0100, a ecrit:
> * i386/i386/locore.h: user vm_offset_t in the recovery_table
> * x86_64/locore.S: fix RECOVERY() location and keep user regs in %rbx,
>   as it seems the convention. This only applies to 32-bit userspace.
> ---
>  i386/i386/locore.h |  4 ++--
>  x86_64/locore.S    | 20 ++++++++++----------
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/i386/i386/locore.h b/i386/i386/locore.h
> index 00da07ad..a8807dbf 100644
> --- a/i386/i386/locore.h
> +++ b/i386/i386/locore.h
> @@ -27,8 +27,8 @@
>   * Fault recovery in copyin/copyout routines.
>   */
>  struct recovery {
> -     int     fault_addr;
> -     int     recover_addr;
> +     vm_offset_t     fault_addr;
> +     vm_offset_t     recover_addr;
>  };
>  
>  extern struct recovery recover_table[];
> diff --git a/x86_64/locore.S b/x86_64/locore.S
> index a2663aff..47d9085c 100644
> --- a/x86_64/locore.S
> +++ b/x86_64/locore.S
> @@ -1157,18 +1157,18 @@ syscall_native:
>       andq    %r10,%r10
>       jz      mach_call_call          /* skip argument copy if none */
>  
> -     movq    R_UESP(%rbx),%rbx       /* get user stack pointer */
> -     addq    $4,%rbx                 /* Skip user return address */
> -
>       movq    $USER_DS,%rdx           /* use user data segment for accesses */
>       mov     %dx,%fs
>       movq    %rsp,%r11               /* save kernel ESP for error recovery */
>  
> +     movq    R_UESP(%rbx),%rbp       /* get user stack pointer */
> +     addq    $4,%rbp                 /* Skip user return address */
> +
>  #define PARAM(reg,ereg) \
> -     RECOVER(mach_call_addr_push) \
>       xorq    %reg,%reg               ;\
> -     movl    %fs:(%rbx),%ereg        /* 1st parameter */ ;\
> -     addq    $4,%rbx                 ;\
> +     RECOVER(mach_call_addr_push) \
> +     movl    %fs:(%rbp),%ereg        /* 1st parameter */ ;\
> +     addq    $4,%rbp                 ;\
>       dec     %r10                    ;\
>       jz      mach_call_call
>  
> @@ -1179,12 +1179,12 @@ syscall_native:
>       PARAM(r8,r8d)                   /* 5th parameter */
>       PARAM(r9,r9d)                   /* 6th parameter */
>  
> -     lea     (%rbx,%r10,4),%rbx      /* point past last argument */
> +     lea     (%rbp,%r10,4),%rbp      /* point past last argument */
>       xorq    %r12,%r12
>  
> -0:   subq    $4,%rbx
> +0:   subq    $4,%rbp
>       RECOVER(mach_call_addr_push)
> -     movl    %fs:(%rbx),%r12d
> +     movl    %fs:(%rbp),%r12d
>       pushq   %r12                    /* push argument on stack */
>       dec     %r10
>       jnz     0b                      /* loop for all arguments */
> @@ -1208,7 +1208,7 @@ mach_call_call:
>  
>  /*
>   * Address out of range.  Change to page fault.
> - * %esi holds failing address.
> + * %rsi holds failing address.
>   */
>  mach_call_addr_push:
>       movq    %r11,%rsp               /* clean parameters from stack */
> -- 
> 2.30.2
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.