This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)



On 20/12/2018 18:46, Vineet Gupta wrote:
> On 12/20/18 12:06 PM, Adhemerval Zanella wrote:
>>
>>
>> On 20/12/2018 17:23, Vineet Gupta wrote:
>>> On 12/20/18 3:19 AM, Adhemerval Zanella wrote:
>>>>>> #define SET_SA_RESTORER(kact, act)                      \
>>>>>>   (kact)->sa_flags = (act)->sa_flags | SA_RESTORER;     \
>>>>>>   (kact)->sa_restorer = &__default_rt_sa_restorer
>>>>> +#define SET_SA_RESTORER(kact, act)				\
>>>>> + ({								\
>>>>> +   if (!((kact)->sa_flags & SA_RESTORER))			\
>>>>> +     {								\
>>>>> +       (kact)->sa_restorer = __default_rt_sa_restorer;		\
>>>>> +       (kact)->sa_flags |= SA_RESTORER;			\
>>>>> +     }								\
>>>>> +   else							\
>>>>> +     (kact)->sa_restorer = (act)->sa_restorer;			\
>>>>> + })
>>>> What is so special about ARC sa_restorer that an application should provide
>>>> an specialized one?
>>>
>>> Its the other way around. Only if application provides it's own restorer, we honor
>>> it, otherwise default restorer is used. This logic goes back many many years ago
>>> to how ARC uClibc did this and likely inherited it from the port it was copied
>>> from. But I don't know if say POSIX allows apps to provide their own restorer or
>>> not. We could very well remove it; although per other discussion if we intend to
>>> use the same struct sigaction for kernel/userland, the placeholder would exist and
>>> we could choose to just ignore it.
>>>
>>
>> The 'should' should be indeed 'might' in my question. And SA_RESTORER is a Linux
>> specific ABI not intended to be used by applications, 
> 
> What do u mean here. It is ABI between userspace and kernel, but not necessarily
> between applications and glibc, although the same "carrier" sa_flags is in play ?
> IOW, SA_RESTORER is not intended to be set by applications, but only by glibc ?

AFAIK SA_RESTORER is meant to be used by libc or alike environments to setup
the required code to handle signal handling, it should not be up to programs
to mess up with sa_restorer to provide custom routines.

> 
>> so my question is in fact
>> what kind of specialized sa_restorer applications might provided that would
>> require the libc to honour it. 
> 
> Indeed, applications should not be allowed to change it. The exact signature of
> default sigreturn is hardwired in signal unwinding etc etc and any deviation from
> that is recipe for issues: but like I mentioned I didn't invent that interface for
> ARC.

So my suggestion is not allow user code to mess with sa_restore regardless 
if sa_flags contain SA_RESTORER.  If kernel allows it to be zero in case of
!SA_RESTORER, set it to zero as other architectures.  If it requires an
special routine, add an arch-specific one within glibc.

> 
>> The placeholder existence is not an issue itself (POSIX states the minimum
>> fields sigaction should provide, not its specific fields neither their
>> layout).
> 
> OK !
> 
>>>> Can't it follow other abi where they issue __NR_sigreturn
>>>> for !SA_SIGINFO or __NR_rt_sigreturn otherwise?
>>>
>>> With Linux UAPI ABI, __NR_sigreturn doesn't exist, but your concern is about
>>> restorer ?
>>
>> Right, so ARC at least is not pulling old compat stuff. Is it safe to just zero
>> the sa_restorer for sa_flags without SA_RESTORER?
> 
> Thing is ARC signal return depends on a restorer. Meaning !SA_RESTORER doesn't
> effectively exist (between libc and kernel). We currently honor user provided
> value, instead we could simply overwrite it with default_rt_restorer and on the
> way out, zero it out to avoid leaking the glibc guts to curious users.
> 

Yes, this is what I suggest ARC should do.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]