Re: git: 889b56c8cd84 - main - setrlimit: Take stack gap into account.
- In reply to: Marcin Wojtas : "Re: git: 889b56c8cd84 - main - setrlimit: Take stack gap into account."
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 18 Oct 2021 13:02:34 UTC
In message <CAPv3WKeP-xFMzwNDVTt9--k0LawzPe8uN8feEBRg_MjX2-Rv7A@mail.gmail.c
om>
, Marcin Wojtas writes:
> Hi Cy,
>
>
>
> sob., 16 paź 2021 o 18:02 Cy Schubert <Cy.Schubert@cschubert.com> napisał(a
> ):
> >
> > 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=889b56c8cd84c9a9f2d9e3b019c1
> 54d6
> > > 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 t
> hey
> > > @@ -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 *e
> ntry
> > > );
> > > 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 *for
> k_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?
> >
> >
>
> At a first glance I don't see the ports' global versioning apart from
> creating tags, but maybe ports' maintainers will have more insight.
I'm also the ntp port maintainer, hence the request. But no worries. I used
the prior __FreeBSD_version bump at the time. There was another
__FreeBSD_version bump yesterday (Sunday). Since this commit is in between
the two I'll update the patch to use the second one and send the patch
upstream.
--
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.