Re: PaX: Heritage bug - Take 2

[Maxime Villard <max%m00nbsd.net@localhost>]

I've committed it.


Le 06/08/2015 17:26, Maxime Villard a écrit :
> Updated diff
> 
> 
> Index: kern/exec_elf.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/exec_elf.c,v
> retrieving revision 1.75
> diff -u -r1.75 exec_elf.c
> --- kern/exec_elf.c	5 Aug 2015 15:58:01 -0000	1.75
> +++ kern/exec_elf.c	6 Aug 2015 15:23:51 -0000
> @@ -116,8 +116,7 @@
>  #define	ELF_TRUNC(a, b)		((a) & ~((b) - 1))
> 
>  static void
> -elf_placedynexec(struct lwp *l, struct exec_package *epp, Elf_Ehdr *eh,
> -    Elf_Phdr *ph)
> +elf_placedynexec(struct exec_package *epp, Elf_Ehdr *eh, Elf_Phdr *ph)
>  {
>  	Elf_Addr align, offset;
>  	int i;
> @@ -127,7 +126,7 @@
>  			align = ph[i].p_align;
> 
>  #ifdef PAX_ASLR
> -	if (pax_aslr_active(l)) {
> +	if (pax_aslr_epp_active(epp)) {
>  		size_t pax_align, l2, delta;
>  		uint32_t r;
> 
> @@ -717,12 +716,8 @@
>  		pos = (Elf_Addr)startp;
>  	}
> 
> -#if defined(PAX_MPROTECT) || defined(PAX_SEGVGUARD) || defined(PAX_ASLR)
> -	pax_setup_elf_flags(l, epp->ep_pax_flags);
> -#endif /* PAX_MPROTECT || PAX_SEGVGUARD || PAX_ASLR */
> -
>  	if (is_dyn)
> -		elf_placedynexec(l, epp, eh, ph);
> +		elf_placedynexec(epp, eh, ph);
> 
>  	/*
>  	 * Load all the necessary sections
> @@ -947,8 +942,15 @@
>  			    np->n_descsz == ELF_NOTE_PAX_DESCSZ &&
>  			    memcmp(ndata, ELF_NOTE_PAX_NAME,
>  			    ELF_NOTE_PAX_NAMESZ) == 0) {
> -				memcpy(&epp->ep_pax_flags, ndesc,
> -				    sizeof(epp->ep_pax_flags));
> +				uint32_t flags;
> +				memcpy(&flags, ndesc, sizeof(flags));
> +#if defined(PAX_MPROTECT) || defined(PAX_SEGVGUARD) || defined(PAX_ASLR)
> +				/* Convert the flags and insert them into
> +				 * the exec package. */
> +				pax_setup_elf_flags(epp, flags);
> +#else
> +				(void)flags; /* UNUSED */
> +#endif /* PAX_MPROTECT || PAX_SEGVGUARD || PAX_ASLR */
>  				break;
>  			}
>  			BADNOTE("PaX tag");
> Index: kern/exec_subr.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/exec_subr.c,v
> retrieving revision 1.71
> diff -u -r1.71 exec_subr.c
> --- kern/exec_subr.c	29 Mar 2014 09:31:11 -0000	1.71
> +++ kern/exec_subr.c	6 Aug 2015 15:23:51 -0000
> @@ -408,7 +408,7 @@
>  	    max_stack_size);
> 
>  #ifdef PAX_ASLR
> -	pax_aslr_stack(l, epp, &max_stack_size);
> +	pax_aslr_stack(epp, &max_stack_size);
>  #endif /* PAX_ASLR */
> 
>  	l->l_proc->p_stackbase = epp->ep_minsaddr;
> Index: kern/kern_exec.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/kern_exec.c,v
> retrieving revision 1.413
> diff -u -r1.413 kern_exec.c
> --- kern/kern_exec.c	31 Jul 2015 07:37:17 -0000	1.413
> +++ kern/kern_exec.c	6 Aug 2015 15:23:51 -0000
> @@ -705,9 +705,9 @@
>  	 */
> 
>  #ifdef PAX_ASLR
> -#define	ASLR_GAP(l)	(pax_aslr_active(l) ? (cprng_fast32() % PAGE_SIZE) : 0)
> +#define	ASLR_GAP(epp)	(pax_aslr_epp_active(epp) ? (cprng_fast32() %
> PAGE_SIZE) : 0)
>  #else
> -#define	ASLR_GAP(l)	0
> +#define	ASLR_GAP(epp)	0
>  #endif
> 
>  #ifdef __MACHINE_STACK_GROWS_UP
> @@ -725,7 +725,7 @@
> 
>  	data->ed_argslen = calcargs(data, argenvstrlen);
> 
> -	const size_t len = calcstack(data, ASLR_GAP(l) + RTLD_GAP);
> +	const size_t len = calcstack(data, ASLR_GAP(epp) + RTLD_GAP);
> 
>  	if (len > epp->ep_ssize) {
>  		/* in effect, compare to initial limit */
> @@ -1097,6 +1097,9 @@
>  	/* Remove POSIX timers */
>  	timers_free(p, TIMERS_POSIX);
> 
> +	/* Set the PaX flags. */
> +	p->p_pax = epp->ep_pax_flags;
> +
>  	/*
>  	 * Do whatever is necessary to prepare the address space
>  	 * for remapping.  Note that this might replace the current
> Index: kern/kern_pax.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/kern_pax.c,v
> retrieving revision 1.31
> diff -u -r1.31 kern_pax.c
> --- kern/kern_pax.c	4 Aug 2015 18:28:09 -0000	1.31
> +++ kern/kern_pax.c	6 Aug 2015 15:23:51 -0000
> @@ -285,7 +285,7 @@
>  }
> 
>  void
> -pax_setup_elf_flags(struct lwp *l, uint32_t elf_flags)
> +pax_setup_elf_flags(struct exec_package *epp, uint32_t elf_flags)
>  {
>  	uint32_t flags = 0;
> 
> @@ -305,7 +305,7 @@
>  	}
>  #endif
> 
> -	l->l_proc->p_pax = flags;
> +	epp->ep_pax_flags = flags;
>  }
> 
>  #if defined(PAX_MPROTECT) || defined(PAX_SEGVGUARD) || defined(PAX_ASLR)
> @@ -372,6 +372,12 @@
>  }
> 
>  bool
> +pax_aslr_epp_active(struct exec_package *epp)
> +{
> +	return pax_flags_active(epp->ep_pax_flags, P_PAX_ASLR);
> +}
> +
> +bool
>  pax_aslr_active(struct lwp *l)
>  {
>  	return pax_flags_active(l->l_proc->p_pax, P_PAX_ASLR);
> @@ -408,9 +414,9 @@
>  }
> 
>  void
> -pax_aslr_stack(struct lwp *l, struct exec_package *epp, u_long
> *max_stack_size)
> +pax_aslr_stack(struct exec_package *epp, u_long *max_stack_size)
>  {
> -	if (!pax_aslr_active(l))
> +	if (!pax_aslr_epp_active(epp))
>  		return;
> 
>  	u_long d = PAX_ASLR_DELTA(cprng_fast32(),
> Index: sys/pax.h
> ===================================================================
> RCS file: /cvsroot/src/sys/sys/pax.h,v
> retrieving revision 1.14
> diff -u -r1.14 pax.h
> --- sys/pax.h	4 Aug 2015 18:28:10 -0000	1.14
> +++ sys/pax.h	6 Aug 2015 15:23:51 -0000
> @@ -50,7 +50,7 @@
>  #endif /* PAX_ASLR */
> 
>  void pax_init(void);
> -void pax_setup_elf_flags(struct lwp *, uint32_t);
> +void pax_setup_elf_flags(struct exec_package *, uint32_t);
>  void pax_adjust(struct lwp *, uint32_t);
> 
>  void pax_mprotect(struct lwp *, vm_prot_t *, vm_prot_t *);
> @@ -58,9 +58,11 @@
> 
>  #define	PAX_ASLR_DELTA(delta, lsb, len)	\
>      (((delta) & ((1UL << (len)) - 1)) << (lsb))
> +
> +bool pax_aslr_epp_active(struct exec_package *);
>  bool pax_aslr_active(struct lwp *);
>  void pax_aslr_init_vm(struct lwp *, struct vmspace *);
> -void pax_aslr_stack(struct lwp *, struct exec_package *, u_long *);
> +void pax_aslr_stack(struct exec_package *, u_long *);
>  void pax_aslr_mmap(struct lwp *, vaddr_t *, vaddr_t, int);
> 
>  #endif /* !_SYS_PAX_H_ */
> 
> 
> 
> 
> 
> 
> 
> 
> Le 03/08/2015 10:53, Maxime Villard a écrit :
>> [I sent this mail some days ago, but it didn't reach tech-kern@.]
>>
>> 	http://marc.info/?l=netbsd-tech-kern&m=142486844004373&w=2
>>
>> Here is a new patch. Now, the PaX flag is converted and registered
>> into the exec package by the loaders (only one loader does that, the
>> native ELF). And right before launching the process, the flag is set
>> in the proc structure - if it fails, the process is aborted, so this
>> is not that bad. In the meantime, all the PaX operations read the
>> exec package's flag instead of the LWP's.
>>
>> Important note, for the record:
>> In exec_subr.c, exec_setup_stack() is called from the loaders, so it
>> must read the exec package's flag. However, the other vmcmd_xx()
>> functions are called when setting up the proc address space, which is
>> done way later in execve_dovmcmds(); and they must therefore read the
>> LWP's flag. A particular order must be followed:
>>  - The loaders set the exec package's PaX flag
>>  - The loaders call exec_setup_stack(), which does not require a PaX
>>    initialization. Which is perfectly fine, since nothing has been
>>    initialized for the moment.
>>  - Here, if the loaders return an error, the calling process's PaX
>>    flag is not overwritten.
>>  - Later, the proc's PaX flag is overwritten. It is set right after
>>    killing the LWPs associated to that proc, and right before calling
>>    pax_aslr_init_vm(). pax_aslr_init_vm() reads the proc's PaX flag,
>>    so that's fine.
>>  - Right after pax_aslr_init_vm(), execve_dovmcmds() is called, and
>>    reads the proc's PaX flag, and expects PaX to be initialized. Which
>>    is perfectly fine too.
>>  - So now everything is ok; and if the execution fails, the process is
>>    aborted.
>>
>> Please review this change before I commit it (if you understood
>> something). Thanks.
>>
>