Re: [PATCH] powerpc: Use sysconf (_SC_SIGSTKSZ) to set SIGSTKSZ and MINSIGSTKSZ.

[Tulio Magno Quites Machado Filho <tuliom@ascii.art.br>]

Manjunath S Matti <mmatti@linux.vnet.ibm.com> writes:

> On 28/04/23 11:35 pm, Tulio Magno Quites Machado Filho wrote:
>> Manjunath Matti via Libc-alpha<libc-alpha@sourceware.org>  writes:
>>
>>> +/* Minimum stack size for a signal handler: SIGSTKSZ/4.  */
>>> +# undef MINSIGSTKSZ
>>> +# define MINSIGSTKSZ (SIGSTKSZ >> 2)
>>> +#endif
>> I didn't understand this part.
>> Why SIGSTKSZ/4 ? I know this is correct now, but I think the kernel is
>> allowed to use another value.
>> Why is this part not using sysconf(_SC_MINSIGSTKSZ)?
>> I'm not suggesting to use sysconf() here, but I'm trying to understand
>> why the same source of value for both SIGSTKSZ and MINSIGSTKSZ is not
>> being used.
> In file: sysdeps/unix/sysv/linux/sysconf-sigstksz.h
>
>   28   if (minsigstacksize < MINSIGSTKSZ)
>   29     minsigstacksize = MINSIGSTKSZ;
>   30   /* MAX (MINSIGSTKSZ, sysconf (_SC_MINSIGSTKSZ)) * 4.  */
>   31   long int sigstacksize = minsigstacksize * 4;
>
> So we are not changing the default implementation.

I'm not sure I understood you. Are you trying to tell me that you want
sysconf_sigstksz() to continue to return the same result?

If this is the case, be careful with the creation of
sysdeps/unix/sysv/linux/powerpc/bits/sigstksz.h because it is an
installed header. That means the values that are being set here will leak
to user code if __USE_DYNAMIC_STACK_SIZE is defined.

If that happens, user code may end up having
MINSIGSTKSZ != getauxval(AT_MINSIGSTKSZ) if the kernel decides to change
the value of AT_MINSIGSTKSZ.

>> If we reach consensus that both macros in this file can have values set
>> at runtime, then I it might be worth adding a test in order to check that
>> dl_minsigstacksize, MINSIGSTKSZ and AT_MINSIGSTKSZ passed by the kernel
>> are identical.
>>
> There are testcases which already use MINSIGSTKSZ
>
> sysdeps/pthread/tst-signal6.c
>
> signal/tst-minsigstksz-5.c

AFAICS none of these tests are checking if dl_minsigstacksize, MINSIGSTKSZ and
AT_MINSIGSTKSZ have identical values.
AFAIU, at least on powerpc, they should always be identical.
Having such a test would help:
- to signal if the kernel changes it without a warning.
- if another commit changed one of them by mistake.

AFAIU, the tests you pointed out help to identify if the current sizes
are indeed enough to handle signals, which is also very important.

-- 
Tulio Magno