[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 10 Jul 2019 17:48:07 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Fix the use of sigaltstack to return to the saved
main stack.
On Wed, Jul 10, 2019 at 11:23:19PM +0200, Szabolcs Nagy wrote:
> * James Y Knight <jyknight@...gle.com> [2019-07-10 16:11:23 -0400]:
> > int sigaltstack(const stack_t *restrict ss, stack_t *restrict old)
> > {
> > + // We must check requirements which Linux fails to verify in the syscall
> > + // itself.
> > if (ss) {
> > - if (ss->ss_size < MINSIGSTKSZ) {
> > + // The syscall does already check against MINSIGSTKSZ, however,
> > + // the kernel's value is smaller than musl's value on some
> > + // architectures. Thus, although this check may appear
> > + // redundant, it is not.
>
> the comment does not make sense to me, the check is obviously
> not redundant.
Yes. Also, in musl, we generally document motivations like this as
part of commit messages rather than comments. This ties them to the
timeline of changes, to the author, and prevents them from sticking
around when code changes and they no longer make sense.
James, could you submit this patch just as the minimal change to
correct the current bug? If additional documentation of why things are
the way they are is needed that can be done separately.
> MINSIGSTKSZ is a libc api, has nothing to do with the kernel
>
> the kernel also defines a MINSIGSZTKSZ but musl is an
> abstraction layer higher, the linux limit should not be
> observable to users, only the limit defined by musl,
> which ensures not only that the kernel can deliver a
> signal but also reserves space of any current or future
> hackery the c runtime may need to do around signal handling,
> so that trivial c language signal handler is guaranteed
> to work.
>
> this is the only reasonable way to make such limit useful.
> if it were only a kernel limit, then application code would
> have to guess the libc signal handling overhead and add that
> to the MINSIGSZTKSZ when allocating signal stacks.
In this case it's more that the kernel values are just wrong. libc
isn't imposing a stronger limit here because of libc code needing
stack, but because the kernel values don't account for signal frame
size. The kernel values presumably can't be changed because the
syscall interface is stable/locked, and it's risky to change for libc
too after it's set (see the issue with whether the x86 values are
right in the presence of AVX512 -- that's why on later archs we
imposed stronger limits).
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.
