Re: git: 889b56c8cd84 - main - setrlimit: Take stack gap into account.

In message <202110150823.19F8NEr9047194@gitrepo.freebsd.org>, Marcin Wojtas 
wri
tes:
> The branch main has been updated by mw:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=889b56c8cd84c9a9f2d9e3b019c154d6
> f14d9021
>
> commit 889b56c8cd84c9a9f2d9e3b019c154d6f14d9021
> Author:     Dawid Gorecki <dgr@semihalf.com>
> AuthorDate: 2021-10-13 19:01:08 +0000
> Commit:     Marcin Wojtas <mw@FreeBSD.org>
> CommitDate: 2021-10-15 08:21:47 +0000
>
>     setrlimit: Take stack gap into account.
>     
>     Calling setrlimit with stack gap enabled and with low values of stack
>     resource limit often caused the program to abort immediately after
>     exiting the syscall. This happened due to the fact that the resource
>     limit was calculated assuming that the stack started at sv_usrstack,
>     while with stack gap enabled the stack is moved by a random number
>     of bytes.
>     
>     Save information about stack size in struct vmspace and adjust the
>     rlim_cur value. If the rlim_cur and stack gap is bigger than rlim_max,
>     then the value is truncated to rlim_max.
>     
>     PR: 253208
>     Reviewed by: kib
>     Obtained from: Semihalf
>     Sponsored by: Stormshield
>     MFC after: 1 month
>     Differential Revision: https://reviews.freebsd.org/D31516
> ---
>  sys/kern/imgact_elf.c    |  5 +++--
>  sys/kern/kern_exec.c     | 11 ++++++++---
>  sys/kern/kern_resource.c |  3 +++
>  sys/sys/imgact_elf.h     |  2 +-
>  sys/sys/sysent.h         |  2 +-
>  sys/vm/vm_map.c          |  2 ++
>  sys/vm/vm_map.h          |  1 +
>  7 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c
> index ef1edfcabaf0..898f0f66a532 100644
> --- a/sys/kern/imgact_elf.c
> +++ b/sys/kern/imgact_elf.c
> @@ -2684,7 +2684,7 @@ __elfN(untrans_prot)(vm_prot_t prot)
>  	return (flags);
>  }
>  
> -void
> +vm_size_t
>  __elfN(stackgap)(struct image_params *imgp, uintptr_t *stack_base)
>  {
>  	uintptr_t range, rbase, gap;
> @@ -2692,7 +2692,7 @@ __elfN(stackgap)(struct image_params *imgp, uintptr_t *
> stack_base)
>  
>  	pct = __elfN(aslr_stack_gap);
>  	if (pct == 0)
> -		return;
> +		return (0);
>  	if (pct > 50)
>  		pct = 50;
>  	range = imgp->eff_stack_sz * pct / 100;
> @@ -2700,4 +2700,5 @@ __elfN(stackgap)(struct image_params *imgp, uintptr_t *
> stack_base)
>  	gap = rbase % range;
>  	gap &= ~(sizeof(u_long) - 1);
>  	*stack_base -= gap;
> +	return (gap);
>  }
> diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
> index 50e75fda6cfb..9dceebdd8441 100644
> --- a/sys/kern/kern_exec.c
> +++ b/sys/kern/kern_exec.c
> @@ -1148,6 +1148,7 @@ exec_new_vmspace(struct image_params *imgp, struct syse
> ntvec *sv)
>  		    stack_prot, error, vm_mmap_to_errno(error));
>  		return (vm_mmap_to_errno(error));
>  	}
> +	vmspace->vm_stkgap = 0;
>  
>  	/*
>  	 * vm_ssize and vm_maxsaddr are somewhat antiquated concepts, but they
> @@ -1493,12 +1494,16 @@ exec_args_get_begin_envv(struct image_args *args)
>  void
>  exec_stackgap(struct image_params *imgp, uintptr_t *dp)
>  {
> +	struct proc *p = imgp->proc;
> +
>  	if (imgp->sysent->sv_stackgap == NULL ||
> -	    (imgp->proc->p_fctl0 & (NT_FREEBSD_FCTL_ASLR_DISABLE |
> +	    (p->p_fctl0 & (NT_FREEBSD_FCTL_ASLR_DISABLE |
>  	    NT_FREEBSD_FCTL_ASG_DISABLE)) != 0 ||
> -	    (imgp->map_flags & MAP_ASLR) == 0)
> +	    (imgp->map_flags & MAP_ASLR) == 0) {
> +		p->p_vmspace->vm_stkgap = 0;
>  		return;
> -	imgp->sysent->sv_stackgap(imgp, dp);
> +	}
> +	p->p_vmspace->vm_stkgap = imgp->sysent->sv_stackgap(imgp, dp);
>  }
>  
>  /*
> diff --git a/sys/kern/kern_resource.c b/sys/kern/kern_resource.c
> index 4c62961e1bc4..b556d4fded51 100644
> --- a/sys/kern/kern_resource.c
> +++ b/sys/kern/kern_resource.c
> @@ -671,6 +671,9 @@ kern_proc_setrlimit(struct thread *td, struct proc *p, u_
> int which,
>  	if (limp->rlim_max < 0)
>  		limp->rlim_max = RLIM_INFINITY;
>  
> +	if (which == RLIMIT_STACK && limp->rlim_cur != RLIM_INFINITY)
> +		limp->rlim_cur += p->p_vmspace->vm_stkgap;
> +
>  	oldssiz.rlim_cur = 0;
>  	newlim = lim_alloc();
>  	PROC_LOCK(p);
> diff --git a/sys/sys/imgact_elf.h b/sys/sys/imgact_elf.h
> index 97383c6eeeb8..294f17c87b6f 100644
> --- a/sys/sys/imgact_elf.h
> +++ b/sys/sys/imgact_elf.h
> @@ -118,7 +118,7 @@ int	__elfN(remove_brand_entry)(Elf_Brandinfo *entry
> );
>  int	__elfN(freebsd_fixup)(uintptr_t *, struct image_params *);
>  int	__elfN(coredump)(struct thread *, struct vnode *, off_t, int);
>  size_t	__elfN(populate_note)(int, void *, void *, size_t, void **);
> -void	__elfN(stackgap)(struct image_params *, uintptr_t *);
> +vm_size_t	__elfN(stackgap)(struct image_params *, uintptr_t *);
>  int	__elfN(freebsd_copyout_auxargs)(struct image_params *, uintptr_t);
>  void	__elfN(puthdr)(struct thread *, void *, size_t, int, size_t, int);
>  void	__elfN(prepare_notes)(struct thread *, struct note_info_list *,
> diff --git a/sys/sys/sysent.h b/sys/sys/sysent.h
> index ad50bf56e87d..ea96c87a79af 100644
> --- a/sys/sys/sysent.h
> +++ b/sys/sys/sysent.h
> @@ -119,7 +119,7 @@ struct sysentvec {
>  	void		(*sv_elf_core_prepare_notes)(struct thread *,
>  			    struct note_info_list *, size_t *);
>  	int		(*sv_imgact_try)(struct image_params *);
> -	void		(*sv_stackgap)(struct image_params *, uintptr_t *);
> +	vm_size_t	(*sv_stackgap)(struct image_params *, uintptr_t *);
>  	int		(*sv_copyout_auxargs)(struct image_params *,
>  			    uintptr_t);
>  	int		sv_minsigstksz;	/* minimum signal stack size */
> diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
> index 1ac4ccf72f11..87a290b998b9 100644
> --- a/sys/vm/vm_map.c
> +++ b/sys/vm/vm_map.c
> @@ -343,6 +343,7 @@ vmspace_alloc(vm_offset_t min, vm_offset_t max, pmap_pini
> t_t pinit)
>  	vm->vm_taddr = 0;
>  	vm->vm_daddr = 0;
>  	vm->vm_maxsaddr = 0;
> +	vm->vm_stkgap = 0;
>  	return (vm);
>  }
>  
> @@ -4265,6 +4266,7 @@ vmspace_fork(struct vmspace *vm1, vm_ooffset_t *fork_ch
> arge)
>  	vm2->vm_taddr = vm1->vm_taddr;
>  	vm2->vm_daddr = vm1->vm_daddr;
>  	vm2->vm_maxsaddr = vm1->vm_maxsaddr;
> +	vm2->vm_stkgap = vm1->vm_stkgap;
>  	vm_map_lock(old_map);
>  	if (old_map->busy)
>  		vm_map_wait_busy(old_map);
> diff --git a/sys/vm/vm_map.h b/sys/vm/vm_map.h
> index ace205b21b42..873ff62eec4a 100644
> --- a/sys/vm/vm_map.h
> +++ b/sys/vm/vm_map.h
> @@ -293,6 +293,7 @@ struct vmspace {
>  	caddr_t vm_taddr;	/* (c) user virtual address of text */
>  	caddr_t vm_daddr;	/* (c) user virtual address of data */
>  	caddr_t vm_maxsaddr;	/* user VA at max stack growth */
> +	vm_size_t vm_stkgap;	/* stack gap size in bytes */
>  	u_int vm_refcnt;	/* number of references */
>  	/*
>  	 * Keep the PMAP last, so that CPU-specific variations of that
>

Is it possible to have a __FreeBSD_version bump for ports?


-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy@nwtime.org>    Web:  https://nwtime.org

	The need of the many outweighs the greed of the few.