[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250505165336.GB1827@brightrain.aerifal.cx>
Date: Mon, 5 May 2025 12:53:36 -0400
From: Rich Felker <dalias@...c.org>
To: alice <alice@...ya.dev>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64
On Mon, May 05, 2025 at 09:46:00AM -0400, Rich Felker wrote:
> On Sat, Aug 31, 2024 at 12:33:55PM -0400, Rich Felker wrote:
> > On Thu, Aug 29, 2024 at 04:23:38PM -0400, Rich Felker wrote:
> > > On Thu, Aug 29, 2024 at 09:11:38PM +0200, alice wrote:
> > > > On Thu Aug 29, 2024 at 9:03 PM CEST, Rich Felker wrote:
> > > > > On Thu, Aug 29, 2024 at 06:00:52PM +0200, alice wrote:
> > > > > > On Thu Aug 29, 2024 at 2:57 PM CEST, Rich Felker wrote:
> > > > > > > On Thu, Aug 29, 2024 at 05:38:42AM +0200, psykose wrote:
> > > > > > > > since kernel commit 2f82ec19757f58549467db568c56e7dfff8af283
> > > > > > > > (https://github.com/torvalds/linux/commit/2f82ec19757f58549467db568c56e7dfff8af283)
> > > > > > > > the kernel has updated these minimum values. having these small values breaks
> > > > > > > > sysconf(_SC_MINSIGSTKSZ) too; it returns 4224 in musl currently which ends up
> > > > > > > > returning ENOMEM from the syscall made in sigaltstack.
> > > > > > > >
> > > > > > > > raising these to match the kernel fixes sigaltstack use on powerpc64(le).
> > > > > > > > caught by glib's 2.82 testsuite
> > > > > > >
> > > > > > > I don't follow how you're claiming sysconf(_SC_MINSIGSTKSZ) is broken..
> > > > > > > It will just return the kernel-provided value on new kernels that
> > > > > > > insist on having a larger stack. In particular I don't see where the
> > > > > > > value 4224 is supposed to be coming from. If there's something I'm
> > > > > > > missing, please explain.
> > > > > >
> > > > > > sysconf(_SC_MINSIGSTKSZ) returns 4224 on ppc64le (this is as far as i know
> > > > > > expected).
> > > > >
> > > > > I don't have a real system handy to test on, so I'm executing this
> > > > > mentally, and not seeing where 4224 comes from.
> > > > > sysconf(_SC_MINSIGSTKSZ) should return the kernel-provided value from
> > > > > __getauxval(AT_MINSIGSTKSZ) unless it's less than the fixed macro
> > > > > value MINSIGSTKSZ. Since that's 4096, the only way I can see this
> > > > > happening is if the kernel filled in AT_MINSIGSTKSZ as 4224, which
> > > > > would be a kernel bug...?
> > > >
> > > > yes, that getauxval gives 4224.
> > > > feel free to forward it to the right place if you think it's a kernel bug :)
> > > >
> > > > (it might just be an oversight since it was coordinated with glibc and so no
> > > > programs ever hit this as glibc made the minimum match the 8192 correctly..)
> > >
> > > Wow, it is a kernel bug:
> > >
> > > https://elixir.bootlin.com/linux/v6.10.6/source/arch/powerpc/kernel/signal_64.c#L69
> > >
> > > So I guess we need a workaround for this. It will prevent the
> > > functionality from working at all, making it so programs always crash
> > > if the kernel needs more than the "default" 8k, because it has no
> > > actual working stack space included, only the size of the signal
> > > frame.
> > >
> > > Fixing this will require coordination with the kernel folks to figure
> > > out if they intend to leave it broken (i.e. if we need to add 3968 on
> > > top of what they tell us via the aux vector) or if they're going to
> > > make a contract that, if the value is >8192, it's the full correct
> > > value for min signal stack size, not just the sigframe.
> > >
> > > BTW this is why I like insisting on actually understanding the source
> > > of a problem rather than just making changes to make it go away. Here
> > > we discovered a much deeper issue that's going to bite folks in the
> > > future.
> >
> > I'm working on the fix for this, but I think one decision needs to be
> > made that I'd like input from ppc folks on:
> >
> > We can either change the definition of the MINSIGSTKSZ macro on
> > powerpc64 (does 32-bit need change too??) or we can add a mechanism
> > for the arch to define an alternate minimum for
> > sysconf(_SC_MINSIGSTKSZ) that might be higher than MINSIGSTKSZ.
> >
> > The former is (probably very minor) "ABI breakage", but I don't think
> > anything would care.
> >
> > Without further fiddling to detect old kernels, either fix *probably*
> > breaks old ppc binaries which are using the MINSIGSTKSZ macro value,
> > even if running on old kernels -- the dynamic sysconf(_SC_MINSIGSTKSZ)
> > limit would always be at least 8k, and since they'd be passing stacks
> > smaller than 8k, sigaltstack would need to fail. (It's not failing
> > now, which is a bug; I'm fixing that because otherwise you'll be able
> > to setup alt stacks that overflow and clobber memory, since the kernel
> > doesn't correctly check the min.)
>
> This was brought up again on IRC; I'd forgotten we hadn't concluded
> the powerpc case here.
>
> I'm still unsure what we should do. What's happened, at least as I
> understand it, is that there was a kernel which quietly increased
> MINSIGSTKSZ. This makes existing binaries built to use the fixed
> MINSIGSTKSZ fail after kernel upgrade, but it also has it so, even
> with commit 8c43c562694fd0436494dc9d3faabb3eea86f9d8 and applications
> using sysconf(_SC_MINSIGSTKSZ), sigaltstack will fail on new kernels
> since the clamping we're doing still uses the old minimum and does not
> ensure the final size is larger than the new minimum.
>
> If we increase the value of MINSIGSTKSZ, which requires (for
> conformance) making sigaltstack enforce the new minimum, existing
> binaries will start failing with upgraded libc.so, even if they're
> still running on a kernel they'd be fine on.
>
> If we leave MINSIGSTKSZ alone and just expose the increased minimum
> through sysconf, the same would happen: sigaltstack uses sysconf, and
> would reject the smaller size, despite it being fine on the existing
> kernel.
>
> A third option that's slightly more complex would be using the new
> minimum only if AT_MINSIGSTKSZ was passed by the kernel, and otherwise
> having sysconf return the old minimum. But I don't think this really
> helps unless AT_MINSIGSTKSZ was added at the same time as the mimimum
> size bump (I haven't yet done the kernel research to find out). If
> not, we'd still have broken cases.
>
> A fourth option, I guess, would be to find some way to probe the
> actual minimum and use that. I don't like this; it'd messy and
> stateful.
>
> Any opinions from ppc folks on how we should proceed?
Looks like we're in luck! AT_MINSIGSTKSZ and the bump in minimum for
powerpc were added in the same pull by Linus,
6112bd00e84e5dbffebc3c1e908cbe914ca772ee, coming from two consecutive
commits, 2f82ec19757f58549467db568c56e7dfff8af283 and
2896b2dff49d0377e4372f470dcddbcb26f2be59.
So I think we can solve this problem, without breaking anything, by
having sysconf(_SC_MINSIGSTKSZ) do the following:
- If AT_MINSIGSTKSZ is not present, use the traditional ABI minimum as
is done now (implicitly as a consequence of not present being same
as zero; that needs to be made explicit)
- If AT_MINSIGSTKSZ is present, use an arch-dependent alternative
minimum to do the clamping, so that we never return a value that
will be rejected by SYS_sigaltstack.
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.
