[PATCH] sigaction: don't sign-extend sa_flags

[Adhemerval Zanella Netto]

On 02/06/25 06:24, Florian Weimer wrote:
> * наб:
> 
>> Before:
>>   rt_sigaction(SIGBUS, {sa_handler=0x55abb9960139, sa_mask=[], sa_flags=SA_RESTORER|SA_RESETHAND|SA_SIGINFO|0xffffffff00000000, sa_restorer=0x7fb1b2a82050}, NULL, 8) = 0
>>
>> After:
>>   rt_sigaction(SIGBUS, {sa_handler=0x7f6a70dce139, sa_mask=[], sa_flags=SA_RESTORER|SA_RESETHAND|SA_SIGINFO, sa_restorer=0x7f6a70c28f60}, NULL, 8) = 0
>>
>> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> 
> I wonder if this is something that should be fixed in strace?
> 
> It's not that the kernel can start using the upper bits with some
> compatibility check (e.g., bits 32 and 33 have to be distinct).

POSIX defines sa_flags is an 'int' [1], while the rt signal interface
uses unsigned (alpha seems to be an outlier, which uses 'int' as well).

So I think it should be fixed on strace as well; the high bits usage would
be possible only on 64-bit ABIs.

[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/sigaction.html

> 
>> ---
>>  sysdeps/unix/sysv/linux/libc_sigaction.c        | 2 +-
>>  sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/sysdeps/unix/sysv/linux/libc_sigaction.c b/sysdeps/unix/sysv/linux/libc_sigaction.c
>> index bbfc177655..f349f8660b 100644
>> --- a/sysdeps/unix/sysv/linux/libc_sigaction.c
>> +++ b/sysdeps/unix/sysv/linux/libc_sigaction.c
>> @@ -49,7 +49,7 @@ __libc_sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
>>      {
>>        kact.k_sa_handler = act->sa_handler;
>>        memcpy (&kact.sa_mask, &act->sa_mask, sizeof (sigset_t));
>> -      kact.sa_flags = act->sa_flags;
>> +      kact.sa_flags = (unsigned)act->sa_flags;
>>        SET_SA_RESTORER (&kact, act);
>>      }
> 
> Our style looks more like this:
> 
>   kact.sa_flags = (unsigned int) act->sa_flags;
> 
>> diff --git a/sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c b/sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c
>> index 006c532127..812e023584 100644
>> --- a/sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c
>> +++ b/sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c
>> @@ -22,7 +22,7 @@
>>  extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
>>  
>>  #define SET_SA_RESTORER(kact, act)			\
>> -  (kact)->sa_flags = (act)->sa_flags | SA_RESTORER;	\
>> +  (kact)->sa_flags |= SA_RESTORER;			\
>>    (kact)->sa_restorer = &restore_rt
>>  
>>  #define RESET_SA_RESTORER(act, kact) 			\
> 
> I looked at the other architectures, and they already use this
> construct.