[PATCH v12 4/8] signal: clear non-uapi flag bits when passing/returning sa_flags
Dave Martin
Dave.Martin at arm.com
Mon Nov 2 12:26:06 EST 2020
On Fri, Oct 16, 2020 at 05:12:29PM -0700, Peter Collingbourne wrote:
> Previously we were not clearing non-uapi flag bits in
> sigaction.sa_flags when storing the userspace-provided sa_flags or
> when returning them via oldact. Start doing so.
>
> This allows userspace to detect missing support for flag bits and
> allows the kernel to use non-uapi bits internally, as we are already
> doing in arch/x86 for two flag bits. Now that this change is in
> place, we no longer need the code in arch/x86 that was hiding these
> bits from userspace, so remove it.
>
> This is technically a userspace-visible behavior change for sigaction, as
> the unknown bits returned via oldact.sa_flags are no longer set. However,
> we are free to define the behavior for unknown bits exactly because
> their behavior is currently undefined, so for now we can define the
> meaning of each of them to be "clear the bit in oldact.sa_flags unless
> the bit becomes known in the future". Furthermore, this behavior is
> consistent with OpenBSD [1], illumos [2] and XNU [3] (FreeBSD [4] and
> NetBSD [5] fail the syscall if unknown bits are set). So there is some
> precedent for this behavior in other kernels, and in particular in XNU,
> which is probably the most popular kernel among those that I looked at,
> which means that this change is less likely to be a compatibility issue.
>
> Link: [1] https://github.com/openbsd/src/blob/f634a6a4b5bf832e9c1de77f7894ae2625e74484/sys/kern/kern_sig.c#L278
> Link: [2] https://github.com/illumos/illumos-gate/blob/76f19f5fdc974fe5be5c82a556e43a4df93f1de1/usr/src/uts/common/syscall/sigaction.c#L86
> Link: [3] https://github.com/apple/darwin-xnu/blob/a449c6a3b8014d9406c2ddbdc81795da24aa7443/bsd/kern/kern_sig.c#L480
> Link: [4] https://github.com/freebsd/freebsd/blob/eded70c37057857c6e23fae51f86b8f8f43cd2d0/sys/kern/kern_sig.c#L699
> Link: [5] https://github.com/NetBSD/src/blob/3365779becdcedfca206091a645a0e8e22b2946e/sys/kern/sys_sig.c#L473
> Signed-off-by: Peter Collingbourne <pcc at google.com>
> Link: https://linux-review.googlesource.com/id/I35aab6f5be932505d90f3b3450c083b4db1eca86
Reviewed-by: Dave Martin <Dave.Martin at arm.com>
> ---
> v10:
> - rename SA_UAPI_FLAGS -> UAPI_SA_FLAGS
> - refactor how we define it to avoid mentioning flags more
> than once
>
> arch/arm/include/asm/signal.h | 2 ++
> arch/parisc/include/asm/signal.h | 2 ++
> arch/x86/kernel/signal_compat.c | 7 -------
> include/linux/signal_types.h | 12 ++++++++++++
> kernel/signal.c | 10 ++++++++++
> 5 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/include/asm/signal.h b/arch/arm/include/asm/signal.h
> index 65530a042009..430be7774402 100644
> --- a/arch/arm/include/asm/signal.h
> +++ b/arch/arm/include/asm/signal.h
> @@ -17,6 +17,8 @@ typedef struct {
> unsigned long sig[_NSIG_WORDS];
> } sigset_t;
>
> +#define __ARCH_UAPI_SA_FLAGS (SA_THIRTYTWO | SA_RESTORER)
> +
> #define __ARCH_HAS_SA_RESTORER
>
> #include <asm/sigcontext.h>
> diff --git a/arch/parisc/include/asm/signal.h b/arch/parisc/include/asm/signal.h
> index 715c96ba2ec8..30dd1e43ef88 100644
> --- a/arch/parisc/include/asm/signal.h
> +++ b/arch/parisc/include/asm/signal.h
> @@ -21,6 +21,8 @@ typedef struct {
> unsigned long sig[_NSIG_WORDS];
> } sigset_t;
>
> +#define __ARCH_UAPI_SA_FLAGS _SA_SIGGFAULT
> +
> #include <asm/sigcontext.h>
>
> #endif /* !__ASSEMBLY */
> diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
> index a7f3e12cfbdb..ddfd919be46c 100644
> --- a/arch/x86/kernel/signal_compat.c
> +++ b/arch/x86/kernel/signal_compat.c
> @@ -165,16 +165,9 @@ void sigaction_compat_abi(struct k_sigaction *act, struct k_sigaction *oact)
> {
> signal_compat_build_tests();
>
> - /* Don't leak in-kernel non-uapi flags to user-space */
> - if (oact)
> - oact->sa.sa_flags &= ~(SA_IA32_ABI | SA_X32_ABI);
> -
> if (!act)
> return;
>
> - /* Don't let flags to be set from userspace */
> - act->sa.sa_flags &= ~(SA_IA32_ABI | SA_X32_ABI);
> -
> if (in_ia32_syscall())
> act->sa.sa_flags |= SA_IA32_ABI;
> if (in_x32_syscall())
> diff --git a/include/linux/signal_types.h b/include/linux/signal_types.h
> index f8a90ae9c6ec..a7887ad84d36 100644
> --- a/include/linux/signal_types.h
> +++ b/include/linux/signal_types.h
> @@ -68,4 +68,16 @@ struct ksignal {
> int sig;
> };
>
> +#ifndef __ARCH_UAPI_SA_FLAGS
> +#ifdef SA_RESTORER
> +#define __ARCH_UAPI_SA_FLAGS SA_RESTORER
> +#else
> +#define __ARCH_UAPI_SA_FLAGS 0
> +#endif
> +#endif
> +
> +#define UAPI_SA_FLAGS \
> + (SA_NOCLDSTOP | SA_NOCLDWAIT | SA_SIGINFO | SA_ONSTACK | SA_RESTART | \
> + SA_NODEFER | SA_RESETHAND | __ARCH_UAPI_SA_FLAGS)
> +
> #endif /* _LINUX_SIGNAL_TYPES_H */
> diff --git a/kernel/signal.c b/kernel/signal.c
> index a38b3edc6851..2ef8c238679e 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3984,6 +3984,16 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
> if (oact)
> *oact = *k;
>
> + /*
> + * Clear unknown flag bits in order to allow userspace to detect missing
> + * support for flag bits and to allow the kernel to use non-uapi bits
> + * internally.
> + */
> + if (act)
> + act->sa.sa_flags &= UAPI_SA_FLAGS;
> + if (oact)
> + oact->sa.sa_flags &= UAPI_SA_FLAGS;
> +
> sigaction_compat_abi(act, oact);
>
> if (act) {
> --
> 2.29.0.rc1.297.gfa9743e501-goog
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
More information about the linux-arm-kernel
mailing list