[musl] [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64

[musl] [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64

[psykose]

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
---
 arch/powerpc64/bits/signal.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
index d5493b18..3b16c4f9 100644
--- a/arch/powerpc64/bits/signal.h
+++ b/arch/powerpc64/bits/signal.h
@@ -2,8 +2,8 @@
  || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
 
 #if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
-#define MINSIGSTKSZ 4096
-#define SIGSTKSZ    10240
+#define MINSIGSTKSZ 8192
+#define SIGSTKSZ    32768
 #endif
 
 #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)

base-commit: dd1e63c3638d5f9afb857fccf6ce1415ca5f1b8b
-- 
2.46.0

Re: [musl] [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64

[Rich Felker]

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.

Changing the macros is ABI breakage, perhaps minor, but still not
nice. My leaning if any change is made would be to remove them, but
unfortunately the Austin Group didn't seem to have gotten the sysconf
stuff (and making these macros optional) into Issue 8, so they're
still mandatory. So I'm not sure what is best to do. Ultimately, due
to kernel bad behavior ignoring the "don't break userspace" rule and
ignoring POSIX, we're in a situation where the macros are required but
necessarily don't actually work, and the only way to make a reliably
working application is to use the not-yet-standard sysconf() lookups
instead.

Regardless of how MINSIGSTKSZ is handled, increasing SIGSTKSZ from 10k
to 32k makes no sense. At most it needs to be increased by the
increase in MINSIGSTKSZ, or perhaps twice that if the goal is to allow
for 2 signal frames (but we currently don't do that in sysconf, so if
that's desirable it should be proposed as its own thing not a
special-case jacking up of the value for one arch).

Rich

Re: [musl] [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64

[alice]

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).

setting stack.ss_size = 4224 (from that call) and passing it to sigaltstack
returns ENOMEM because it is smaller than 8192 which the kernel enforces.

..so the normal way of using sigaltstack with the smallest size (with sysconf or
the macros) is broken.
making the MINSIGSTKSZ match the actual value the kernel enforces fixes it.

Re: [musl] [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64

[Rich Felker]

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...?

> setting stack.ss_size = 4224 (from that call) and passing it to sigaltstack
> returns ENOMEM because it is smaller than 8192 which the kernel enforces.
> 
> ...so the normal way of using sigaltstack with the smallest size (with sysconf or
> the macros) is broken.
> making the MINSIGSTKSZ match the actual value the kernel enforces fixes it.

What the kernel enforces varies by version and possibly also hardware
capabilities.

Rich

Re: [musl] [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64

[alice]

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..)

> > setting stack.ss_size = 4224 (from that call) and passing it to sigaltstack
> > returns ENOMEM because it is smaller than 8192 which the kernel enforces.
> > 
> > ...so the normal way of using sigaltstack with the smallest size (with sysconf or
> > the macros) is broken.
> > making the MINSIGSTKSZ match the actual value the kernel enforces fixes it.
>
> What the kernel enforces varies by version and possibly also hardware
> capabilities.

evidently it's enforcing exactly what that commit says that minsigstksz is (now)
on ppc64*, since setting ss_size to 8191 gives ENOMEM and setting it to 8192
works.

you can wait for a hypothetical fix to __getauxval(AT_MINSIGSTKSZ) to also
return 8192 if you want, but programs that use the macro defined value will
remain broken if it isn't changed and clearly that is the minimum value on new
kernels :)

i'll keep the patch downstream in any case

>
> Rich

Re: [musl] [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64

[Rich Felker]

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.

Rich

Re: [musl] [PATCH] fix MINSIGSTKSZ and SIGSTKSZ for powerpc64

[Rich Felker]

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.)

Rich