From: Nick Desaulniers <ndesaulniers@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: clang-built-linux@googlegroups.com,
Al Viro <viro@zeniv.linux.org.uk>,
Oleg Nesterov <oleg@redhat.com>,
Nathan Chancellor <natechancellor@gmail.com>,
Nick Desaulniers <ndesaulniers@google.com>,
Will Deacon <will@kernel.org>,
Michael Ellerman <mpe@ellerman.id.au>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Willem de Bruijn <willemb@google.com>,
Xiaoming Ni <nixiaoming@huawei.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Peter Collingbourne <pcc@google.com>,
Kees Cook <keescook@chromium.org>, Jens Axboe <axboe@kernel.dk>,
Christian Brauner <christian.brauner@ubuntu.com>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org
Subject: [PATCH] x86: signal: Don't do sas_ss_reset() until we are certain that sigframe won't be abandoned
Date: Thu, 22 Apr 2021 16:08:37 -0700 [thread overview]
Message-ID: <20210422230846.1756380-1-ndesaulniers@google.com> (raw)
From: Al Viro <viro@zeniv.linux.org.uk>
Currently we handle SS_AUTODISARM as soon as we have stored the
altstack settings into sigframe - that's the point when we have
set the things up for eventual sigreturn to restore the old settings.
And if we manage to set the sigframe up (we are not done with that
yet), everything's fine. However, in case of failure we end up
with sigframe-to-be abandoned and SIGSEGV force-delivered. And
in that case we end up with inconsistent rules - late failures
have altstack reset, early ones do not.
It's trivial to get consistent behaviour - just handle SS_AUTODISARM
once we have set the sigframe up and are committed to entering
the handler, i.e. in signal_delivered().
Link: https://lore.kernel.org/lkml/20200404170604.GN23230@ZenIV.linux.org.uk/
Link: https://github.com/ClangBuiltLinux/linux/issues/876
Acked-by: Oleg Nesterov <oleg@redhat.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
include/linux/compat.h | 2 --
include/linux/signal.h | 2 --
kernel/signal.c | 14 ++++----------
3 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 6e65be753603..40ad060deb82 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -494,8 +494,6 @@ int __compat_save_altstack(compat_stack_t __user *, unsigned long);
&__uss->ss_sp, label); \
unsafe_put_user(t->sas_ss_flags, &__uss->ss_flags, label); \
unsafe_put_user(t->sas_ss_size, &__uss->ss_size, label); \
- if (t->sas_ss_flags & SS_AUTODISARM) \
- sas_ss_reset(t); \
} while (0);
/*
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 205526c4003a..20887ff4c27a 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -460,8 +460,6 @@ int __save_altstack(stack_t __user *, unsigned long);
unsafe_put_user((void __user *)t->sas_ss_sp, &__uss->ss_sp, label); \
unsafe_put_user(t->sas_ss_flags, &__uss->ss_flags, label); \
unsafe_put_user(t->sas_ss_size, &__uss->ss_size, label); \
- if (t->sas_ss_flags & SS_AUTODISARM) \
- sas_ss_reset(t); \
} while (0);
#ifdef CONFIG_PROC_FS
diff --git a/kernel/signal.c b/kernel/signal.c
index f2718350bf4b..384030909daf 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2815,6 +2815,8 @@ static void signal_delivered(struct ksignal *ksig, int stepping)
if (!(ksig->ka.sa.sa_flags & SA_NODEFER))
sigaddset(&blocked, ksig->sig);
set_current_blocked(&blocked);
+ if (current->sas_ss_flags & SS_AUTODISARM)
+ sas_ss_reset(current);
tracehook_signal_handler(stepping);
}
@@ -4133,11 +4135,7 @@ int __save_altstack(stack_t __user *uss, unsigned long sp)
int err = __put_user((void __user *)t->sas_ss_sp, &uss->ss_sp) |
__put_user(t->sas_ss_flags, &uss->ss_flags) |
__put_user(t->sas_ss_size, &uss->ss_size);
- if (err)
- return err;
- if (t->sas_ss_flags & SS_AUTODISARM)
- sas_ss_reset(t);
- return 0;
+ return err;
}
#ifdef CONFIG_COMPAT
@@ -4192,11 +4190,7 @@ int __compat_save_altstack(compat_stack_t __user *uss, unsigned long sp)
&uss->ss_sp) |
__put_user(t->sas_ss_flags, &uss->ss_flags) |
__put_user(t->sas_ss_size, &uss->ss_size);
- if (err)
- return err;
- if (t->sas_ss_flags & SS_AUTODISARM)
- sas_ss_reset(t);
- return 0;
+ return err;
}
#endif
base-commit: 16fc44d6387e260f4932e9248b985837324705d8
--
2.31.1.498.g6c1eba8ee3d-goog
reply other threads:[~2021-04-22 23:08 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210422230846.1756380-1-ndesaulniers@google.com \
--to=ndesaulniers@google.com \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=christian.brauner@ubuntu.com \
--cc=christophe.leroy@csgroup.eu \
--cc=clang-built-linux@googlegroups.com \
--cc=ebiederm@xmission.com \
--cc=gustavoars@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mpe@ellerman.id.au \
--cc=natechancellor@gmail.com \
--cc=nixiaoming@huawei.com \
--cc=oleg@redhat.com \
--cc=pcc@google.com \
--cc=tglx@linutronix.de \
--cc=viro@zeniv.linux.org.uk \
--cc=will@kernel.org \
--cc=willemb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).