Re: [PATCH] elf-load: Respect PT_GNU_STACK

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le jeu. 28 mars 2024 09:40:50 +0300, a ecrit:
> On Wed, Mar 27, 2024 at 9:37 PM Samuel Thibault <samuel.thibault@gnu.org> 
> wrote:
> > But it's not getting used anywhere?
> 
> Indeed, I forgot to extract the kern/bootstrap.c part of the change.
> Ooops :) Thanks for pointing it out.
> 
> Sergey
> 
> -- >8 --
> 
> If a bootstrap ELF contains a PT_GNU_STACK phdr, take stack protection
> from there.  Otherwise, default to VM_PROT_ALL.
> ---
>  include/mach/exec/elf.h  | 1 +
>  include/mach/exec/exec.h | 2 ++
>  kern/bootstrap.c         | 8 ++++----
>  kern/elf-load.c          | 7 +++++++
>  4 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/include/mach/exec/elf.h b/include/mach/exec/elf.h
> index 9e4f8f7e..55304496 100644
> --- a/include/mach/exec/elf.h
> +++ b/include/mach/exec/elf.h
> @@ -300,6 +300,7 @@ typedef struct {
>  #define PT_NOTE              4
>  #define PT_SHLIB     5
>  #define PT_PHDR              6
> +#define PT_GNU_STACK 0x6474e551
>  
>  #define PT_LOPROC    0x70000000
>  #define PT_HIPROC    0x7fffffff
> diff --git a/include/mach/exec/exec.h b/include/mach/exec/exec.h
> index 94b234b0..29fa897d 100644
> --- a/include/mach/exec/exec.h
> +++ b/include/mach/exec/exec.h
> @@ -51,6 +51,8 @@ typedef struct exec_info
>       /* (ELF) Address of interpreter string for loading shared libraries, 
> null if none.  */
>       vm_offset_t interp;
>  
> +     /* Required stack protection.  */
> +     vm_prot_t stack_prot;
>  } exec_info_t;
>  
>  typedef int exec_sectype_t;
> diff --git a/kern/bootstrap.c b/kern/bootstrap.c
> index 49358ac6..0470e1b6 100644
> --- a/kern/bootstrap.c
> +++ b/kern/bootstrap.c
> @@ -620,10 +620,10 @@ build_args_and_stack(struct exec_info *boot_exec_info,
>       stack_size = round_page(STACK_SIZE);
>       stack_base = user_stack_low(stack_size);
>  
> -     (void) vm_allocate(current_task()->map,
> -                     &stack_base,
> -                     stack_size,
> -                     FALSE);
> +     (void) vm_map(current_map(), &stack_base, stack_size,
> +                   0, FALSE, IP_NULL, 0, FALSE,
> +                   boot_exec_info->stack_prot, VM_PROT_ALL,
> +                   VM_INHERIT_DEFAULT);
>  
>       arg_pos = (char *)
>               set_user_regs(stack_base, stack_size, boot_exec_info, arg_len);
> diff --git a/kern/elf-load.c b/kern/elf-load.c
> index ce86327c..596233a8 100644
> --- a/kern/elf-load.c
> +++ b/kern/elf-load.c
> @@ -73,6 +73,8 @@ int exec_load(exec_read_func_t *read, exec_read_exec_func_t 
> *read_exec,
>       if (actual < phsize)
>               return EX_CORRUPT;
>  
> +     out_info->stack_prot = VM_PROT_ALL;
> +
>       for (i = 0; i < x.e_phnum; i++)
>       {
>               ph = (Elf_Phdr *)((vm_offset_t)phdr + i * x.e_phentsize);
> @@ -89,6 +91,11 @@ int exec_load(exec_read_func_t *read, 
> exec_read_exec_func_t *read_exec,
>                                             ph->p_vaddr + loadbase, 
> ph->p_memsz, type);
>                       if (result)
>                               return result;
> +             } else if (ph->p_type == PT_GNU_STACK) {
> +                     out_info->stack_prot = 0;
> +                     if (ph->p_flags & PF_R) out_info->stack_prot |= 
> VM_PROT_READ;
> +                     if (ph->p_flags & PF_W) out_info->stack_prot |= 
> VM_PROT_WRITE;
> +                     if (ph->p_flags & PF_X) out_info->stack_prot |= 
> VM_PROT_EXECUTE;
>               }
>       }
>  
> -- 
> 2.44.0
> 

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