Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 12 Oct 2020 05:42:06 -0700
From: "H.J. Lu" <hjl.tools@...il.com>
To: Dave Martin <Dave.Martin@....com>
Cc: Szabolcs Nagy <szabolcs.nagy@....com>, GNU C Library <libc-alpha@...rceware.org>, 
	libc-coord@...ts.openwall.com
Subject: V4 [PATCH] sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305]

On Mon, Oct 12, 2020 at 4:04 AM Dave Martin <Dave.Martin@....com> wrote:
>
> On Mon, Oct 12, 2020 at 08:53:32AM +0100, Szabolcs Nagy via Libc-alpha wrote:
> > The 10/10/2020 05:19, H.J. Lu via Libc-alpha wrote:
> > > Add _SC_MINSIGSTKSZ for the minimum signal stack size derived from
> > > AT_MINSIGSTKSZ, which is the minimum number of bytes of free stack
> > > space required in order to gurantee successful, non-nested handling
> > > of a single signal whose handler is an empty function, and _SC_SIGSTKSZ
> > > which is the suggested minimum number of bytes of stack space required
> > > for a signal stack.
> > >
> > > If AT_MINSIGSTKSZ isn't available, sysconf (_SC_MINSIGSTKSZ) returns
> > > MINSIGSTKSZ.  On Linux/x86 with XSAVE, the signal frame used by kernel
> > > is composed of the following areas and laid out as:
> > >
> > >  ------------------------------
> > >  | alignment padding          |
> > >  ------------------------------
> > >  | xsave buffer               |
> > >  ------------------------------
> > >  | fsave header (32-bit only) |
> > >  ------------------------------
> > >  | siginfo + ucontext         |
> > >  ------------------------------
> > >
> > > Compute AT_MINSIGSTKSZ value as size of xsave buffer + size of fsave
> > > header (32-bit only) + size of siginfo and ucontext + alignment padding.
> > >
> > > If _SC_SIGSTKSZ_SOURCE is defined, MINSIGSTKSZ and SIGSTKSZ are redefined
> > > as
> > >
> > > /* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ).  */
> > >  # undef SIGSTKSZ
> > >  # define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
> > >
> > > /* Minimum stack size for a signal handler: SIGSTKSZ.  */
> > >  # undef MINSIGSTKSZ
> > >  # define MINSIGSTKSZ SIGSTKSZ
> > >
> > > Compilation will fail if the source assumes constant MINSIGSTKSZ or
> > > SIGSTKSZ.
> > >
> > > The reason for not simply increasing the kernel's MINSIGSTKSZ #define
> > > (apart from the fact that it is rarely used, due to glibc's shadowing
> > > definitions) was that userspace binaries will have baked in the old
> > > value of the constant and may be making assumptions about it.
> > >
> > > For example, the type (char [MINSIGSTKSZ]) changes if this #define
> > > changes.  This could be a problem if an newly built library tries to
> > > memcpy() or dump such an object defined by and old binary.
> > > Bounds-checking and the stack sizes passed to things like sigaltstack()
> > > and makecontext() could similarly go wrong.
> >
> >
> > this looks reasonable to me.
> >
> > i added libc-coord on cc as it seems to be
> > a useful generic api across targets.
> >
> >
> > /* Return MAX (MINSIGSTKSZ, sysconf (_SC_MINSIGSTKSZ)) * 4.  */
> > this can be excessive for sigstksz,
> > but reasonable on glibc given the
> > overhead of libc internal signals
> > and lazy binding.
>
> Interesting points.  Can we put actual numbers on those?
>
> Code that tries to allocate correctly sized stacks would need this
> information.
>
> In the presence of things like IFUNC, I suppose there might be no hard
> limit on the amount of stack that might be required to resolve a
> symbol, but I hope most IFUNC functions are pretty minimal.
>
> > does this decrease the size on any
> > existing target?
>
> To avoid unpleasant surprises, I think we should explicitly clamp both
> parameters to be no less than the value of the legacy #define.  Then
> the answer becomes "no" by construction.  Allowing them to be smaller
> will likely save little memory, so it's probably not worth the risk it.

I added sysconf-sigstksz.h:

static long int
sysconf_sigstksz (void)
{
  long int minsigstacksize = GLRO(dl_minsigstacksize);
  assert (minsigstacksize != 0);
  _Static_assert (__builtin_constant_p (MINSIGSTKSZ),
  "MINSIGSTKSZ is constant");
  if (minsigstacksize < MINSIGSTKSZ)
    minsigstacksize = MINSIGSTKSZ;
  /* MAX (MINSIGSTKSZ, sysconf (_SC_MINSIGSTKSZ)) * 4.  */
  long int sigstacksize = minsigstacksize * 4;
  /* Return MAX (SIGSTKSZ, sigstacksize).  */
  _Static_assert (__builtin_constant_p (SIGSTKSZ),
  "SIGSTKSZ is constant");
  if (sigstacksize < SIGSTKSZ)
    sigstacksize = SIGSTKSZ;
  return sigstacksize;
}

A target can override it.

> (The same rule doesn't apply to AT_MINSIGSTKSZ though, since that's a
> new invention, and purely a property of the kernel.  We yield the "true"
> kernel value there, which can be less then MINSIGSTKSZ.)
>
>
> On ia64, we should probably override the default "* 4" rule and keep the
> old value.  There seems zero chance that this architecture will be
> extended with additional register state, and this avoids making the
> current huge value even larger.

IA64 has

/* Minimum stack size for a signal handler.  */
#define MINSIGSTKSZ 4096

/* System default stack size.  */
#define SIGSTKSZ 16384

On ia64,  SIGSTKSZ is unchanged with and without
-D_SC_SIGSTKSZ_SOURCE.  If needed, ia64 can override
bits/sigstksz.h to keep MINSIGSTKSZ and SIGSTKSZ as is.

-- 
H.J.

Download attachment "0001-sysconf-Add-_SC_MINSIGSTKSZ-_SC_SIGSTKSZ-BZ-20305.patch" of type "application/x-patch" (26130 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.