[1/5] m68k: Fix sigaction kernel definition (BZ #23960)
Commit Message
On 12/12/2018 08:41, Andreas Schwab wrote:
> 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.
>
Updated patch below.
--
[BZ #23960]
* sysdeps/unix/sysv/linux/kernel_sigaction.h (HAS_SA_RESTORER):
Define if SA_RESTORER is defined.
(kernel_sigaction): Define sa_restorer if HAS_SA_RESTORER is defined.
(SET_SA_RESTORER, RESET_SA_RESTORER): Define iff the macro are not
already defined.
* sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (SA_RESTORER,
kernel_sigaction, SET_SA_RESTORER, RESET_SA_RESTORER): Remove
definitions.
(HAS_SA_RESTORER): Define.
* sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h (SA_RESTORER,
SET_SA_RESTORER, RESET_SA_RESTORER): Remove definition.
(HAS_SA_RESTORER): Define.
* sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h: Include generic
kernel_sigaction after define SET_SA_RESTORER and RESET_SA_RESTORER.
* sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h: Likewise.
* sysdeps/unix/sysv/linux/s390/kernel_sigaction.h: Likewise.
* sysdeps/unix/sysv/linux/x86_64/sigaction.c: Likewise.
---
Comments
On Dez 12 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> [BZ #23960]
> * sysdeps/unix/sysv/linux/kernel_sigaction.h (HAS_SA_RESTORER):
> Define if SA_RESTORER is defined.
> (kernel_sigaction): Define sa_restorer if HAS_SA_RESTORER is defined.
> (SET_SA_RESTORER, RESET_SA_RESTORER): Define iff the macro are not
> already defined.
> * sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (SA_RESTORER,
> kernel_sigaction, SET_SA_RESTORER, RESET_SA_RESTORER): Remove
> definitions.
> (HAS_SA_RESTORER): Define.
> * sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h (SA_RESTORER,
> SET_SA_RESTORER, RESET_SA_RESTORER): Remove definition.
> (HAS_SA_RESTORER): Define.
> * sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h: Include generic
> kernel_sigaction after define SET_SA_RESTORER and RESET_SA_RESTORER.
> * sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h: Likewise.
> * sysdeps/unix/sysv/linux/s390/kernel_sigaction.h: Likewise.
> * sysdeps/unix/sysv/linux/x86_64/sigaction.c: Likewise.
Ok.
Andreas.
Hi,
On 2018-12-12 16:52, Adhemerval Zanella wrote:
>
>
> On 12/12/2018 08:41, Andreas Schwab wrote:
> > 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.
> >
>
> Updated patch below.
>
> --
>
> [BZ #23960]
> * sysdeps/unix/sysv/linux/kernel_sigaction.h (HAS_SA_RESTORER):
> Define if SA_RESTORER is defined.
> (kernel_sigaction): Define sa_restorer if HAS_SA_RESTORER is defined.
> (SET_SA_RESTORER, RESET_SA_RESTORER): Define iff the macro are not
> already defined.
> * sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (SA_RESTORER,
> kernel_sigaction, SET_SA_RESTORER, RESET_SA_RESTORER): Remove
> definitions.
> (HAS_SA_RESTORER): Define.
> * sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h (SA_RESTORER,
> SET_SA_RESTORER, RESET_SA_RESTORER): Remove definition.
> (HAS_SA_RESTORER): Define.
> * sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h: Include generic
> kernel_sigaction after define SET_SA_RESTORER and RESET_SA_RESTORER.
> * sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h: Likewise.
> * sysdeps/unix/sysv/linux/s390/kernel_sigaction.h: Likewise.
> * sysdeps/unix/sysv/linux/x86_64/sigaction.c: Likewise.
[ snip ]
> diff --git a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
> index 7a6a2c4f29..14f44c200b 100644
> --- a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
> +++ b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
> @@ -30,3 +30,5 @@ struct kernel_sigaction
> (kact)->sa_restorer = (act)->sa_restorer
> #define RESET_SA_RESTORER(act, kact) \
> (act)->sa_restorer = (kact)->sa_restorer
> +
> +#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
This is not correct, you should also remove struct kernel_sigaction from
this file, like it was done in the initial version of the patchset
(patch 5).
Aurelien
On 16/12/2018 18:06, Aurelien Jarno wrote:
> Hi,
>
> On 2018-12-12 16:52, Adhemerval Zanella wrote:
>>
>>
>> On 12/12/2018 08:41, Andreas Schwab wrote:
>>> 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.
>>>
>>
>> Updated patch below.
>>
>> --
>>
>> [BZ #23960]
>> * sysdeps/unix/sysv/linux/kernel_sigaction.h (HAS_SA_RESTORER):
>> Define if SA_RESTORER is defined.
>> (kernel_sigaction): Define sa_restorer if HAS_SA_RESTORER is defined.
>> (SET_SA_RESTORER, RESET_SA_RESTORER): Define iff the macro are not
>> already defined.
>> * sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (SA_RESTORER,
>> kernel_sigaction, SET_SA_RESTORER, RESET_SA_RESTORER): Remove
>> definitions.
>> (HAS_SA_RESTORER): Define.
>> * sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h (SA_RESTORER,
>> SET_SA_RESTORER, RESET_SA_RESTORER): Remove definition.
>> (HAS_SA_RESTORER): Define.
>> * sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h: Include generic
>> kernel_sigaction after define SET_SA_RESTORER and RESET_SA_RESTORER.
>> * sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h: Likewise.
>> * sysdeps/unix/sysv/linux/s390/kernel_sigaction.h: Likewise.
>> * sysdeps/unix/sysv/linux/x86_64/sigaction.c: Likewise.
>
> [ snip ]
>
>> diff --git a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
>> index 7a6a2c4f29..14f44c200b 100644
>> --- a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
>> +++ b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
>> @@ -30,3 +30,5 @@ struct kernel_sigaction
>> (kact)->sa_restorer = (act)->sa_restorer
>> #define RESET_SA_RESTORER(act, kact) \
>> (act)->sa_restorer = (kact)->sa_restorer
>> +
>> +#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
>
> This is not correct, you should also remove struct kernel_sigaction from
> this file, like it was done in the initial version of the patchset
> (patch 5).
>
> Aurelien
>
Thanks for catching it, this is an mistake with my local rebase. I remove
this change for s390 kernel_sigaction.h for this patch.
@@ -1,19 +1,27 @@
#ifndef _KERNEL_SIGACTION_H
# define _KERNEL_SIGACTION_H
+#ifdef SA_RESTORER
+# define HAS_SA_RESTORER 1
+#endif
+
/* This is the sigaction structure from the Linux 3.2 kernel. */
struct kernel_sigaction
{
__sighandler_t k_sa_handler;
unsigned long sa_flags;
-#ifdef SA_RESTORER
+#ifdef HAS_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;
};
-#ifndef SA_RESTORER
+#ifndef SET_SA_RESTORER
# define SET_SA_RESTORER(kact, act)
+#endif
+#ifndef RESET_SA_RESTORER
# define RESET_SA_RESTORER(act, kact)
#endif
@@ -1,22 +1,4 @@
-#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
+/* m68k does not define SA_RESTORER, but does have sa_restorer member
+ on kernel sigaction struct. */
+#define HAS_SA_RESTORER 1
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
@@ -1,8 +1,9 @@
/* NIOS2 uses the generic Linux UAPI but defines SA_RESTORER. */
#define SA_RESTORER 0x04000000
-#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
+
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
@@ -1,9 +1,10 @@
/* powerpc kernel sigaction is similar to generic Linux UAPI one,
but the architecture also defines SA_RESTORER. */
#define SA_RESTORER 0x04000000
-#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
+
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
@@ -30,3 +30,5 @@ struct kernel_sigaction
(kact)->sa_restorer = (act)->sa_restorer
#define RESET_SA_RESTORER(act, kact) \
(act)->sa_restorer = (kact)->sa_restorer
+
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
@@ -1,8 +1,9 @@
/* SH uses the generic Linux UAPI but defines SA_RESTORER. */
#define SA_RESTORER 0x04000000
-#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
+
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
@@ -1,10 +1,5 @@
/* SPARC 'struct __new_sigaction' is similar to generic Linux UAPI with
a sa_restorer field, even though function is passed as an argument
to rt_sigaction syscall. */
-#define SA_RESTORER 0x04000000
+#define HAS_SA_RESTORER 1
#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
-
-#define SET_SA_RESTORER(kact, act) \
- (kact)->sa_restorer = NULL
-#define RESET_SA_RESTORER(act, kact) \
- (act)->sa_restorer = (kact)->sa_restorer
@@ -18,7 +18,6 @@
#include <signal.h>
#define SA_RESTORER 0x04000000
-#include <kernel_sigaction.h>
extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
@@ -29,6 +28,8 @@ extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
#define RESET_SA_RESTORER(act, kact) \
(act)->sa_restorer = (kact)->sa_restorer
+#include <kernel_sigaction.h>
+
#include <sysdeps/unix/sysv/linux/sigaction.c>
/* NOTE: Please think twice before making any changes to the bits of