| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20171206133058.GB6933@osiris>
Date: Wed, 6 Dec 2017 14:30:58 +0100
From: Heiko Carstens <heiko.carstens@...ibm.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Ben Hutchings <ben.hutchings@...ethink.co.uk>,
linux-kernel@...r.kernel.org, stable@...r.kernel.org,
Christian Borntraeger <borntraeger@...ibm.com>,
Martin Schwidefsky <schwidefsky@...ibm.com>
Subject: Re: [PATCH 4.4 02/96] s390/runtime instrumention: fix possible
memory corruption
On Wed, Dec 06, 2017 at 08:44:53AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Dec 05, 2017 at 07:15:34PM +0100, Heiko Carstens wrote:
> > On Tue, Dec 05, 2017 at 06:08:47PM +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 05, 2017 at 05:02:32PM +0000, Ben Hutchings wrote:
> > > > On Tue, 2017-11-28 at 11:22 +0100, Greg Kroah-Hartman wrote:
> > > > > 4.4-stable review patch. If anyone has any objections, please let me know.
> > > > >
> > > > > ------------------
> > > > >
> > > > > From: Heiko Carstens <heiko.carstens@...ibm.com>
> > > > >
> > > > > commit d6e646ad7cfa7034d280459b2b2546288f247144 upstream.
> > > > [...]
> > > > > --- a/arch/s390/kernel/runtime_instr.c
> > > > > +++ b/arch/s390/kernel/runtime_instr.c
> > > > > @@ -47,11 +47,13 @@ void exit_thread_runtime_instr(void)
> > > > > {
> > > > > struct task_struct *task = current;
> > > > >
> > > > > + preempt_disable();
> > > > > if (!task->thread.ri_cb)
> > > > > return;
> > > >
> > > > This return path now leaves preemption disabled. This seems to have
> > > > been fixed upstream by commit 8d9047f8b967 "s390/runtime
> > > > instrumentation: simplify task exit handling".
> > >
> > > "simplify" doesn't seem to imply "fixes a bug" :)
> >
> > Indeed ;) That where two subsequent patches, but incorrectly split by me...
> >
> > > Heiko, should I also queue this patch up?
> >
> > Yes, please.
>
> It doesn't apply to 4.9-stable or 4.4-stable, can you provide a working
> backport?
Below is the patch against 4.4-stable:
>From e3cd188d023506d4a0045b9a2918b9fa73d4d007 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@...ibm.com>
Date: Mon, 11 Sep 2017 11:24:22 +0200
Subject: [PATCH] s390/runtime instrumentation: simplify task exit handling
commit 8d9047f8b967ce6181fd824ae922978e1b055cc0 upstream.
Free data structures required for runtime instrumentation from
arch_release_task_struct(). This allows to simplify the code a bit,
and also makes the semantics a bit easier: arch_release_task_struct()
is never called from the task that is being removed.
In addition this allows to get rid of exit_thread() in a later patch.
Signed-off-by: Heiko Carstens <heiko.carstens@...ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@...ibm.com>
---
arch/s390/include/asm/runtime_instr.h | 4 +++-
arch/s390/kernel/process.c | 2 +-
arch/s390/kernel/runtime_instr.c | 30 +++++++++++++++---------------
3 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/arch/s390/include/asm/runtime_instr.h b/arch/s390/include/asm/runtime_instr.h
index 402ad6df4897..c54a9310d814 100644
--- a/arch/s390/include/asm/runtime_instr.h
+++ b/arch/s390/include/asm/runtime_instr.h
@@ -85,6 +85,8 @@ static inline void restore_ri_cb(struct runtime_instr_cb *cb_next,
load_runtime_instr_cb(&runtime_instr_empty_cb);
}
-void exit_thread_runtime_instr(void);
+struct task_struct;
+
+void runtime_instr_release(struct task_struct *tsk);
#endif /* _RUNTIME_INSTR_H */
diff --git a/arch/s390/kernel/process.c b/arch/s390/kernel/process.c
index efa035a31b98..7bc4e4c5d5b8 100644
--- a/arch/s390/kernel/process.c
+++ b/arch/s390/kernel/process.c
@@ -72,7 +72,6 @@ extern void kernel_thread_starter(void);
*/
void exit_thread(void)
{
- exit_thread_runtime_instr();
}
void flush_thread(void)
@@ -87,6 +86,7 @@ void arch_release_task_struct(struct task_struct *tsk)
{
/* Free either the floating-point or the vector register save area */
kfree(tsk->thread.fpu.regs);
+ runtime_instr_release(tsk);
}
int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
diff --git a/arch/s390/kernel/runtime_instr.c b/arch/s390/kernel/runtime_instr.c
index 70cdb03d4acd..fd03a7569e10 100644
--- a/arch/s390/kernel/runtime_instr.c
+++ b/arch/s390/kernel/runtime_instr.c
@@ -18,11 +18,24 @@
/* empty control block to disable RI by loading it */
struct runtime_instr_cb runtime_instr_empty_cb;
+void runtime_instr_release(struct task_struct *tsk)
+{
+ kfree(tsk->thread.ri_cb);
+}
+
static void disable_runtime_instr(void)
{
- struct pt_regs *regs = task_pt_regs(current);
+ struct task_struct *task = current;
+ struct pt_regs *regs;
+ if (!task->thread.ri_cb)
+ return;
+ regs = task_pt_regs(task);
+ preempt_disable();
load_runtime_instr_cb(&runtime_instr_empty_cb);
+ kfree(task->thread.ri_cb);
+ task->thread.ri_cb = NULL;
+ preempt_enable();
/*
* Make sure the RI bit is deleted from the PSW. If the user did not
@@ -43,19 +56,6 @@ static void init_runtime_instr_cb(struct runtime_instr_cb *cb)
cb->valid = 1;
}
-void exit_thread_runtime_instr(void)
-{
- struct task_struct *task = current;
-
- preempt_disable();
- if (!task->thread.ri_cb)
- return;
- disable_runtime_instr();
- kfree(task->thread.ri_cb);
- task->thread.ri_cb = NULL;
- preempt_enable();
-}
-
SYSCALL_DEFINE1(s390_runtime_instr, int, command)
{
struct runtime_instr_cb *cb;
@@ -64,7 +64,7 @@ SYSCALL_DEFINE1(s390_runtime_instr, int, command)
return -EOPNOTSUPP;
if (command == S390_RUNTIME_INSTR_STOP) {
- exit_thread_runtime_instr();
+ disable_runtime_instr();
return 0;
}
--
2.13.5
Powered by blists - more mailing lists