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 03/05/23 11:18 pm, Tulio Magno Quites Machado Filho wrote:
>> 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?
> Do you want me to implement a powerpc specific function ?
>> 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.
>
> My observation is that, MINSIGSTKSZ is not the same as 
> getauxval(AT_MINSIGSTKSZ).

OK.  And I'm trying to warn you there is a risk of having
MINSIGSTKSZ < getauxval(AT_MINSIGSTKSZ) when __USE_DYNAMIC_STACK_SIZE
is defined.

I'm afraid we're diverging from the original discussion, which is:
the minimum stack size for a signal handler is calculated from the
amount of data the kernel needs to save in the stack.

The kernel calculates that and provide it via getauxval(AT_MINSIGSTKSZ).
AFAIU, calculating the minimum stack size for a signal handler based on
getauxval(AT_SIGSTKSZ) may lead to errors because there are no
guarantees that getauxval(AT_SIGSTKSZ)/4 > getauxval(AT_MINSIGSTKSZ),
even if that is true now, it isn't future proof.

Besides that, the test I suggested to implement would guarantee that
glibc code remains up-to-date according to the interpretation that is
adopted.

Thanks for elaborating your explanation!
That was really helpful.

-- 
Tulio Magno