Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250505134600.GZ1827@brightrain.aerifal.cx>
Date: Mon, 5 May 2025 09:46:00 -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 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?

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.