[1/5] m68k: Fix sigaction kernel definition (BZ #23960)

Message ID 55c8ffc0-c2af-4bc8-8377-3c3310213a83@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Dec. 12, 2018, 10:26 a.m. UTC
  On 11/12/2018 19:26, Andreas Schwab wrote:
> On Dez 11 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> diff --git a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
>> index 54972feb13..eef4bb9b65 100644
>> --- a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
>> +++ b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
>> @@ -1,22 +1,8 @@
>> -#ifndef _KERNEL_SIGACTION_H
>> -# define _KERNEL_SIGACTION_H
>> -
>> -#include <signal.h>
>> -
>> +/* m68k uses the generic Linux UAPI but defines SA_RESTORER.  */
>>  #define SA_RESTORER 0x04000000
>> -
>> -/* This is the sigaction structure from the Linux 3.2 kernel.  */
>> -struct kernel_sigaction
>> -{
>> -  __sighandler_t k_sa_handler;
>> -  sigset_t sa_mask;
>> -  unsigned long sa_flags;
>> -  void (*sa_restorer) (void);
>> -};
>> +#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
>>  
>>  #define SET_SA_RESTORER(kact, act)			\
>>    (kact)->sa_restorer = (act)->sa_restorer
>>  #define RESET_SA_RESTORER(act, kact)			\
>>    (act)->sa_restorer = (kact)->sa_restorer
>> -
>> -#endif
> 
> There should be no need to read or set sa_restorer.  The kernel does not
> use it, and it also doesn't define SA_RESTORER.

The m68k kernel does define SA_RESTORER on arch/m68k/include/asm/signal.h
(__ARCH_HAS_SA_RESTORER), but the only difference it makes for m68k is at
flush_signal_handlers where kernel sets the sa_restorer to NULL. So it
indeed should be safe to just use default kernel_sigaction.h (I don't see
any signal test issues). Updated patch below.

--

	James Clarke  <jrtc27@jrtc27.com>
	Adhemerval Zanella  <adhemerval.zanella@linaro.org>

	[BZ #23960]
	* sysdeps/unix/sysv/linux/kernel_sigaction.h (kernel_sigaction): Add
	comment about the difference due glibc definition.
	* sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h: Remove file.

---
  

Comments

Andreas Schwab Dec. 12, 2018, 10:41 a.m. UTC | #1
On Dez 12 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> The m68k kernel does define SA_RESTORER on arch/m68k/include/asm/signal.h
> (__ARCH_HAS_SA_RESTORER),

These are not the same.  The latter only tells <linux/signal_types.h> to
add the sa_restorer member, independent of SA_RESTORER (which m68k
doesn't define).

> but the only difference it makes for m68k is at flush_signal_handlers
> where kernel sets the sa_restorer to NULL. So it indeed should be safe
> to just use default kernel_sigaction.h (I don't see any signal test
> issues). Updated patch below.

This fails to add the sa_restorer member, causing the signal mask to be
mishandled.

Andreas.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/kernel_sigaction.h b/sysdeps/unix/sysv/linux/kernel_sigaction.h
index 2dbec08099..b7359054b2 100644
--- a/sysdeps/unix/sysv/linux/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/kernel_sigaction.h
@@ -9,6 +9,8 @@  struct kernel_sigaction
 #ifdef SA_RESTORER
   void (*sa_restorer) (void);
 #endif
+  /* glibc sigset is larger than kernel expected one, however sigaction
+     passes the kernel expected size on rt_sigaction syscall.  */
   sigset_t sa_mask;
 };
 
diff --git a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
deleted file mode 100644
index 54972feb13..0000000000
--- a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
+++ /dev/null
@@ -1,22 +0,0 @@ 
-#ifndef _KERNEL_SIGACTION_H
-# define _KERNEL_SIGACTION_H
-
-#include <signal.h>
-
-#define SA_RESTORER 0x04000000
-
-/* This is the sigaction structure from the Linux 3.2 kernel.  */
-struct kernel_sigaction
-{
-  __sighandler_t k_sa_handler;
-  sigset_t sa_mask;
-  unsigned long sa_flags;
-  void (*sa_restorer) (void);
-};
-
-#define SET_SA_RESTORER(kact, act)			\
-  (kact)->sa_restorer = (act)->sa_restorer
-#define RESET_SA_RESTORER(act, kact)			\
-  (act)->sa_restorer = (kact)->sa_restorer
-
-#endif