Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 26 Jul 2018 18:07:47 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: pthread_setattr_default_np behavior difference from glibc

On Thu, Jul 26, 2018 at 11:51:54AM -0400, Rich Felker wrote:
> On Sat, Aug 12, 2017 at 12:38:53AM -0500, Bobby Bingham wrote:
> > I've got a glibc-linked binary I'm trying to get running, and I'm
> > running into the usual stack overflow for threads other than the main
> > thread because of musl's smaller default stack size.  So I decided to
> > see if I could LD_PRELOAD a library with a constructor that used
> > pthread_setattr_default_np to increase the default stack size.
> > 
> > It turns out that it doesn't work for this binary because it passes a
> > thread attribute object to pthread_create without explicitly setting the
> > stack size in the attribute object.  According to the man page, the
> > values set with pthread_getattr_default_np are only used if no attribute
> > object is passed to pthread_create, which is how musl behaves.
> > 
> > It turns out this isn't what glibc does for stack size (but is for guard
> > size).
> > 
> > glibc's pthread_attr_init sets the stack size field in the attribute
> > object to zero, which is interpreted to mean the default stack size at
> > whatever time it's queried.  The attached program will print two
> > different stack sizes on glibc.
> > 
> > I think that the glibc behavior is problematic because
> > pthread_attr_getstacksize can return a value different than what will
> > actually be used if the default is changed before pthread_create is
> > called.
> > 
> > However, if we changed pthread_attr_init to initialize the stack size
> > based on the current default at the time its called, we'd be a little
> > closer to glibc's behavior, but without the quirk I mentioned above.
> > And it would fix my use case :)
> > 
> > Thoughts?
> 
> Somehow I missed this at the time, or forgot about it, and just
> rediscovered the same issue. Actually I thought glibc ignored the
> default set by pthread_setattr_default_np in this case, because I
> wasn't aware of the (likely non-conforming, unless
> pthread_attr_getstacksize patches it up) way zero is used in glibc,
> and I was about to propose we diverge from glibc here just because the
> current behavior is not useful for the intended purpose.
> 
> The whole reason pthread_setattr_default_np was added was to allow
> easy workarounds for applications/libraries that assume large stacks:
> so packagers can just add an early call to pthread_setattr_default_np
> to ensure that all threads created have sufficiently large stacks. But
> some code, notably SDL, creates an attribute object to set other
> properties, leaving the stack size alone, and thereby gets the
> musl-default stack size rather than the new default configured by
> pthread_setattr_default_np.
> 
> The attached patch fixes this, and I think I'll apply it, but I'm not
> entirely happy with the situation before or after this change, since
> pthread_setattr_default_np is thread-unsafe and thus it's unsafe to
> call from ctors in libraries that might be dynamic-loaded or where
> threads might have already been created by other libraries' ctors.
> 
> I think we should probably have pthread_setattr_default_np use a_cas
> to update the defaults, changing them from size_t to volatile int
> (there's no need to support defaults >INT_MAX; doing so would just be
> harmful) so a_cas is usable on them. Then the consumers,
> pthread_create and pthread_attr_init, would be getting relaxed-order
> atomics rather than data races. Assuming there's something else to
> order the pthread_setattr_default_np with respect to pthread_create or
> pthread_attr_init (e.g. being called in the same thread) you know the
> latter will get the latest-set default; otherwise it gets *some valid*
> default from the past or present.

One possible additional improvement that could be made:

There's a PT_GNU_STACK ELF program header that's normally only used to
request non-executable stack, but it can also specify a stack size the
program wants. This is used (and was historically required) for
FDPIC/NOMMU targets to tell the kernel how much stack to reserve for
the main thread, and is conventionally left at 0/default for normal
ELF, but ld is capable of writing a custom size with
-Wl,-z,stacksize=X.

So, we could automatically increase __default_stacksize according to
max(current,dso.pt_gnu_stacksize.p_memsz) for the main program and
each loaded dso. This would make it possible to work around programs
that need large stacks but don't explicitly request them without
patching, by instead just adding the appropriate LDFLAGS.

Are there any significant cons to doing this? Would it help
distros/integrators?

Rich

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.