From nobody Mon Oct 18 13:10:11 2021
X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1])
	by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 06E731813D7D;
	Mon, 18 Oct 2021 13:10:14 +0000 (UTC)
	(envelope-from cy.schubert@cschubert.com)
Received: from omta002.cacentral1.a.cloudfilter.net (omta002.cacentral1.a.cloudfilter.net [3.97.99.33])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "Client", Issuer "CA" (not verified))
	by mx1.freebsd.org (Postfix) with ESMTPS id 4HXy0P66SCz4gWg;
	Mon, 18 Oct 2021 13:10:13 +0000 (UTC)
	(envelope-from cy.schubert@cschubert.com)
Received: from shw-obgw-4004a.ext.cloudfilter.net ([10.228.9.227])
	by cmsmtp with ESMTP
	id c6DimkAQWps7PcSP7mGSyG; Mon, 18 Oct 2021 13:10:13 +0000
Received: from spqr.komquats.com ([70.66.148.124])
	by cmsmtp with ESMTPA
	id cSP6m7oVva8XRcSP6mmUDZ; Mon, 18 Oct 2021 13:10:13 +0000
X-Authority-Analysis: v=2.4 cv=Ov8sdwzt c=1 sm=1 tr=0 ts=616d7235
 a=Cwc3rblV8FOMdVN/wOAqyQ==:117 a=Cwc3rblV8FOMdVN/wOAqyQ==:17
 a=kj9zAlcOel0A:10 a=8gfv0ekSlNoA:10 a=YxBL1-UpAAAA:8 a=6I5d2MoRAAAA:8
 a=oCJs8q-oAAAA:8 a=EkcXrb_YAAAA:8 a=lYWpZslvAE-qNA2gE9QA:9 a=CjuIK1q_8ugA:10
 a=UJ0tAi3fqDAA:10 a=Ia-lj3WSrqcvXOmTRaiG:22 a=IjZwj45LgO3ly-622nXo:22
 a=qUF70SbvcHBaGhGVny9j:22 a=LK5xJRSDVpKd5WXXoEvA:22
Received: from slippy.cwsent.com (slippy [10.1.1.91])
	by spqr.komquats.com (Postfix) with ESMTPS id A2200263;
	Mon, 18 Oct 2021 06:10:11 -0700 (PDT)
Received: from slippy (localhost [127.0.0.1])
	by slippy.cwsent.com (8.16.1/8.16.1) with ESMTP id 19IDABYd005908;
	Mon, 18 Oct 2021 06:10:11 -0700 (PDT)
	(envelope-from Cy.Schubert@cschubert.com)
Message-Id: <202110181310.19IDABYd005908@slippy.cwsent.com>
X-Mailer: exmh version 2.9.0 11/07/2018 with nmh-1.7.1
Reply-to: Cy Schubert <Cy.Schubert@cschubert.com>
From: Cy Schubert <Cy.Schubert@cschubert.com>
X-os: FreeBSD
X-Sender: cy@cwsent.com
X-URL: http://www.cschubert.com/
To: Warner Losh <imp@bsdimp.com>
cc: Cy Schubert <Cy.Schubert@cschubert.com>, Marcin Wojtas <mw@freebsd.org>,
        src-committers <src-committers@freebsd.org>,
        "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>,
        dev-commits-src-main@freebsd.org
Subject: Re: git: 889b56c8cd84 - main - setrlimit: Take stack gap into 
 account.
In-reply-to: <CANCZdfrCPSoKeKGfuQETGv=9WYLiCBTcZv2XQ-6uC99Q1Jvp5A@mail.gmail.com>
References: <202110150823.19F8NEr9047194@gitrepo.freebsd.org> 
 <202110161602.19GG2FYs004292@slippy.cwsent.com> <CANCZdfrCPSoKeKGfuQETGv=9WYLiCBTcZv2XQ-6uC99Q1Jvp5A@mail.gmail.com>
Comments: In-reply-to Warner Losh <imp@bsdimp.com>
   message dated "Mon, 18 Oct 2021 07:03:29 -0600."
List-Id: Commit messages for the main branch of the src repository <dev-commits-src-main.freebsd.org>
List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main
List-Help: <mailto:dev-commits-src-main+help@freebsd.org>
List-Post: <mailto:dev-commits-src-main@freebsd.org>
List-Subscribe: <mailto:dev-commits-src-main+subscribe@freebsd.org>
List-Unsubscribe: <mailto:dev-commits-src-main+unsubscribe@freebsd.org>
Sender: owner-dev-commits-src-main@freebsd.org
X-BeenThere: dev-commits-src-main@freebsd.org
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Date: Mon, 18 Oct 2021 06:10:11 -0700
X-CMAE-Envelope: MS4xfHOq2+XhK3NvqIqSNoPxGrc7cG2ZWu0LATSbjfiOT+pHENzlcqKU4h4z003n4OZ0ouV3NZGTultcQwlEWDvoAE78BCz0ABuJTi/cu3owZT/lgcc35pGF
 NUji0TDndOWbaPaZ85Ewo/09ZfxkW/GN80M8IUsjXnM7iryqHEhAmfHfiu1NAzJ1DO/+Zb3LAQ4diEpxxYMNmHRNUjjp4sCAdB4RgwhR9jgjkiwcQlVMGaH3
 GGEkvFK6hlZoGRtS/aWHBgMGU+BNgMVYzNac2P50yQi0uxoLlBFJD7ZKBk1ZoadP0rCohYf57mO8Qc0A27CKNO9xDrrk3vxP4SswKSLHZJVon4c5kIzEvbJ6
 gGzsXCgf
X-Rspamd-Queue-Id: 4HXy0P66SCz4gWg
X-Spamd-Bar: ----
Authentication-Results: mx1.freebsd.org;
	none
X-Spamd-Result: default: False [-4.00 / 15.00];
	 REPLY(-4.00)[]
X-ThisMailContainsUnwantedMimeParts: N

In message <CANCZdfrCPSoKeKGfuQETGv=9WYLiCBTcZv2XQ-6uC99Q1Jvp5A@mail.gmail.c
om>
, Warner Losh writes:
> On Sat, Oct 16, 2021, 10:02 AM Cy Schubert <Cy.Schubert@cschubert.com>
> wrote:
>
> > 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?
> >
>
> There was a bump for linuxkpi you should use for this. It was a day or so
> after the stackgap change.

That's the one I intend to use. I used the prior one at the time but will 
update the patch to use this one instead. Probably today sometime.


-- 
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.