* [PATCH v5 09/10] powerpc/signal64: Rewrite rt_sigreturn() to minimise uaccess switches
From: Christopher M. Riedl @ 2021-02-03 18:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Axtens
In-Reply-To: <20210203184323.20792-1-cmr@codefail.de>
From: Daniel Axtens <dja@axtens.net>
Add uaccess blocks and use the 'unsafe' versions of functions doing user
access where possible to reduce the number of times uaccess has to be
opened/closed.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Co-developed-by: Christopher M. Riedl <cmr@codefail.de>
Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
---
arch/powerpc/kernel/signal_64.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index a471e97589a8..817b64e1e409 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -782,9 +782,13 @@ SYSCALL_DEFINE0(rt_sigreturn)
* restore_tm_sigcontexts.
*/
regs->msr &= ~MSR_TS_MASK;
+#endif
- if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR]))
+ if (!user_read_access_begin(uc, sizeof(*uc)))
goto badframe;
+
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ unsafe_get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR], badframe_block);
#endif
if (MSR_TM_ACTIVE(msr)) {
@@ -794,10 +798,12 @@ SYSCALL_DEFINE0(rt_sigreturn)
/* Trying to start TM on non TM system */
if (!cpu_has_feature(CPU_FTR_TM))
- goto badframe;
+ goto badframe_block;
+
+ unsafe_get_user(uc_transact, &uc->uc_link, badframe_block);
+
+ user_read_access_end();
- if (__get_user(uc_transact, &uc->uc_link))
- goto badframe;
if (restore_tm_sigcontexts(current, &uc->uc_mcontext,
&uc_transact->uc_mcontext))
goto badframe;
@@ -816,12 +822,9 @@ SYSCALL_DEFINE0(rt_sigreturn)
* causing a TM bad thing.
*/
current->thread.regs->msr &= ~MSR_TS_MASK;
- if (!user_read_access_begin(uc, sizeof(*uc)))
- return -EFAULT;
- if (__unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext)) {
- user_read_access_end();
- goto badframe;
- }
+ unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext,
+ badframe_block);
+
user_read_access_end();
}
@@ -831,6 +834,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
set_thread_flag(TIF_RESTOREALL);
return 0;
+badframe_block:
+ user_read_access_end();
badframe:
signal_fault(current, regs, "rt_sigreturn", uc);
--
2.26.1
^ permalink raw reply related
* [PATCH v5 06/10] powerpc/signal64: Replace setup_sigcontext() w/ unsafe_setup_sigcontext()
From: Christopher M. Riedl @ 2021-02-03 18:43 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20210203184323.20792-1-cmr@codefail.de>
Previously setup_sigcontext() performed a costly KUAP switch on every
uaccess operation. These repeated uaccess switches cause a significant
drop in signal handling performance.
Rewrite setup_sigcontext() to assume that a userspace write access window
is open. Replace all uaccess functions with their 'unsafe' versions
which avoid the repeated uaccess switches.
Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
---
arch/powerpc/kernel/signal_64.c | 70 ++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 8e1d804ce552..4248e4489ff1 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -101,9 +101,13 @@ static void prepare_setup_sigcontext(struct task_struct *tsk, int ctx_has_vsx_re
* Set up the sigcontext for the signal frame.
*/
-static long setup_sigcontext(struct sigcontext __user *sc,
- struct task_struct *tsk, int signr, sigset_t *set,
- unsigned long handler, int ctx_has_vsx_region)
+#define unsafe_setup_sigcontext(sc, tsk, signr, set, handler, \
+ ctx_has_vsx_region, e) \
+ unsafe_op_wrap(__unsafe_setup_sigcontext(sc, tsk, signr, set, \
+ handler, ctx_has_vsx_region), e)
+static long notrace __unsafe_setup_sigcontext(struct sigcontext __user *sc,
+ struct task_struct *tsk, int signr, sigset_t *set,
+ unsigned long handler, int ctx_has_vsx_region)
{
/* When CONFIG_ALTIVEC is set, we _always_ setup v_regs even if the
* process never used altivec yet (MSR_VEC is zero in pt_regs of
@@ -118,20 +122,19 @@ static long setup_sigcontext(struct sigcontext __user *sc,
#endif
struct pt_regs *regs = tsk->thread.regs;
unsigned long msr = regs->msr;
- long err = 0;
/* Force usr to alway see softe as 1 (interrupts enabled) */
unsigned long softe = 0x1;
BUG_ON(tsk != current);
#ifdef CONFIG_ALTIVEC
- err |= __put_user(v_regs, &sc->v_regs);
+ unsafe_put_user(v_regs, &sc->v_regs, efault_out);
/* save altivec registers */
if (tsk->thread.used_vr) {
/* Copy 33 vec registers (vr0..31 and vscr) to the stack */
- err |= __copy_to_user(v_regs, &tsk->thread.vr_state,
- 33 * sizeof(vector128));
+ unsafe_copy_to_user(v_regs, &tsk->thread.vr_state,
+ 33 * sizeof(vector128), efault_out);
/* set MSR_VEC in the MSR value in the frame to indicate that sc->v_reg)
* contains valid data.
*/
@@ -140,12 +143,12 @@ static long setup_sigcontext(struct sigcontext __user *sc,
/* We always copy to/from vrsave, it's 0 if we don't have or don't
* use altivec.
*/
- err |= __put_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33]);
+ unsafe_put_user(tsk->thread.vrsave, (u32 __user *)&v_regs[33], efault_out);
#else /* CONFIG_ALTIVEC */
- err |= __put_user(0, &sc->v_regs);
+ unsafe_put_user(0, &sc->v_regs, efault_out);
#endif /* CONFIG_ALTIVEC */
/* copy fpr regs and fpscr */
- err |= copy_fpr_to_user(&sc->fp_regs, tsk);
+ unsafe_copy_fpr_to_user(&sc->fp_regs, tsk, efault_out);
/*
* Clear the MSR VSX bit to indicate there is no valid state attached
@@ -160,24 +163,27 @@ static long setup_sigcontext(struct sigcontext __user *sc,
*/
if (tsk->thread.used_vsr && ctx_has_vsx_region) {
v_regs += ELF_NVRREG;
- err |= copy_vsx_to_user(v_regs, tsk);
+ unsafe_copy_vsx_to_user(v_regs, tsk, efault_out);
/* set MSR_VSX in the MSR value in the frame to
* indicate that sc->vs_reg) contains valid data.
*/
msr |= MSR_VSX;
}
#endif /* CONFIG_VSX */
- err |= __put_user(&sc->gp_regs, &sc->regs);
+ unsafe_put_user(&sc->gp_regs, &sc->regs, efault_out);
WARN_ON(!FULL_REGS(regs));
- err |= __copy_to_user(&sc->gp_regs, regs, GP_REGS_SIZE);
- err |= __put_user(msr, &sc->gp_regs[PT_MSR]);
- err |= __put_user(softe, &sc->gp_regs[PT_SOFTE]);
- err |= __put_user(signr, &sc->signal);
- err |= __put_user(handler, &sc->handler);
+ unsafe_copy_to_user(&sc->gp_regs, regs, GP_REGS_SIZE, efault_out);
+ unsafe_put_user(msr, &sc->gp_regs[PT_MSR], efault_out);
+ unsafe_put_user(softe, &sc->gp_regs[PT_SOFTE], efault_out);
+ unsafe_put_user(signr, &sc->signal, efault_out);
+ unsafe_put_user(handler, &sc->handler, efault_out);
if (set != NULL)
- err |= __put_user(set->sig[0], &sc->oldmask);
+ unsafe_put_user(set->sig[0], &sc->oldmask, efault_out);
- return err;
+ return 0;
+
+efault_out:
+ return -EFAULT;
}
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -664,12 +670,15 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
if (old_ctx != NULL) {
prepare_setup_sigcontext(current, ctx_has_vsx_region);
- if (!access_ok(old_ctx, ctx_size)
- || setup_sigcontext(&old_ctx->uc_mcontext, current, 0, NULL, 0,
- ctx_has_vsx_region)
- || __copy_to_user(&old_ctx->uc_sigmask,
- ¤t->blocked, sizeof(sigset_t)))
+ if (!user_write_access_begin(old_ctx, ctx_size))
return -EFAULT;
+
+ unsafe_setup_sigcontext(&old_ctx->uc_mcontext, current, 0, NULL,
+ 0, ctx_has_vsx_region, efault_out);
+ unsafe_copy_to_user(&old_ctx->uc_sigmask, ¤t->blocked,
+ sizeof(sigset_t), efault_out);
+
+ user_write_access_end();
}
if (new_ctx == NULL)
return 0;
@@ -698,6 +707,10 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
/* This returns like rt_sigreturn */
set_thread_flag(TIF_RESTOREALL);
return 0;
+
+efault_out:
+ user_write_access_end();
+ return -EFAULT;
}
@@ -850,9 +863,12 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
} else {
err |= __put_user(0, &frame->uc.uc_link);
prepare_setup_sigcontext(tsk, 1);
- err |= setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
- NULL, (unsigned long)ksig->ka.sa.sa_handler,
- 1);
+ if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
+ return -EFAULT;
+ err |= __unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk,
+ ksig->sig, NULL,
+ (unsigned long)ksig->ka.sa.sa_handler, 1);
+ user_write_access_end();
}
err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
if (err)
--
2.26.1
^ permalink raw reply related
* [PATCH v5 08/10] powerpc/signal64: Rewrite handle_rt_signal64() to minimise uaccess switches
From: Christopher M. Riedl @ 2021-02-03 18:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Axtens
In-Reply-To: <20210203184323.20792-1-cmr@codefail.de>
From: Daniel Axtens <dja@axtens.net>
Add uaccess blocks and use the 'unsafe' versions of functions doing user
access where possible to reduce the number of times uaccess has to be
opened/closed.
There is no 'unsafe' version of copy_siginfo_to_user, so move it
slightly to allow for a "longer" uaccess block.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Co-developed-by: Christopher M. Riedl <cmr@codefail.de>
Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
---
arch/powerpc/kernel/signal_64.c | 54 +++++++++++++++++++++------------
1 file changed, 34 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index d668f8af18fe..a471e97589a8 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -849,44 +849,51 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
unsigned long msr = regs->msr;
frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
- if (!access_ok(frame, sizeof(*frame)))
- goto badframe;
- err |= __put_user(&frame->info, &frame->pinfo);
- err |= __put_user(&frame->uc, &frame->puc);
- err |= copy_siginfo_to_user(&frame->info, &ksig->info);
- if (err)
+ /* This only applies when calling unsafe_setup_sigcontext() and must be
+ * called before opening the uaccess window.
+ */
+ if (!MSR_TM_ACTIVE(msr))
+ prepare_setup_sigcontext(tsk, 1);
+
+ if (!user_write_access_begin(frame, sizeof(*frame)))
goto badframe;
+ unsafe_put_user(&frame->info, &frame->pinfo, badframe_block);
+ unsafe_put_user(&frame->uc, &frame->puc, badframe_block);
+
/* Create the ucontext. */
- err |= __put_user(0, &frame->uc.uc_flags);
- err |= __save_altstack(&frame->uc.uc_stack, regs->gpr[1]);
+ unsafe_put_user(0, &frame->uc.uc_flags, badframe_block);
+ unsafe_save_altstack(&frame->uc.uc_stack, regs->gpr[1], badframe_block);
if (MSR_TM_ACTIVE(msr)) {
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* The ucontext_t passed to userland points to the second
* ucontext_t (for transactional state) with its uc_link ptr.
*/
- err |= __put_user(&frame->uc_transact, &frame->uc.uc_link);
+ unsafe_put_user(&frame->uc_transact, &frame->uc.uc_link, badframe_block);
+
+ user_write_access_end();
+
err |= setup_tm_sigcontexts(&frame->uc.uc_mcontext,
&frame->uc_transact.uc_mcontext,
tsk, ksig->sig, NULL,
(unsigned long)ksig->ka.sa.sa_handler,
msr);
+
+ if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
+ goto badframe;
+
#endif
} else {
- err |= __put_user(0, &frame->uc.uc_link);
- prepare_setup_sigcontext(tsk, 1);
- if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
- return -EFAULT;
- err |= __unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk,
- ksig->sig, NULL,
- (unsigned long)ksig->ka.sa.sa_handler, 1);
- user_write_access_end();
+ unsafe_put_user(0, &frame->uc.uc_link, badframe_block);
+ unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
+ NULL, (unsigned long)ksig->ka.sa.sa_handler,
+ 1, badframe_block);
}
- err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
- if (err)
- goto badframe;
+
+ unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
+ user_write_access_end();
/* Make sure signal handler doesn't get spurious FP exceptions */
tsk->thread.fp_state.fpscr = 0;
@@ -901,6 +908,11 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
regs->nip = (unsigned long) &frame->tramp[0];
}
+
+ /* Save the siginfo outside of the unsafe block. */
+ if (copy_siginfo_to_user(&frame->info, &ksig->info))
+ goto badframe;
+
/* Allocate a dummy caller frame for the signal handler. */
newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
err |= put_user(regs->gpr[1], (unsigned long __user *)newsp);
@@ -940,6 +952,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
return 0;
+badframe_block:
+ user_write_access_end();
badframe:
signal_fault(current, regs, "handle_rt_signal64", frame);
--
2.26.1
^ permalink raw reply related
* [PATCH v5 01/10] powerpc/uaccess: Add unsafe_copy_from_user
From: Christopher M. Riedl @ 2021-02-03 18:43 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20210203184323.20792-1-cmr@codefail.de>
Just wrap __copy_tofrom_user() for the usual 'unsafe' pattern which
takes in a label to goto on error.
Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
---
arch/powerpc/include/asm/uaccess.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 501c9a79038c..036e82eefac9 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -542,6 +542,9 @@ user_write_access_begin(const void __user *ptr, size_t len)
#define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
#define unsafe_put_user(x, p, e) __put_user_goto(x, p, e)
+#define unsafe_copy_from_user(d, s, l, e) \
+ unsafe_op_wrap(__copy_tofrom_user((__force void __user *)d, s, l), e)
+
#define unsafe_copy_to_user(d, s, l, e) \
do { \
u8 __user *_dst = (u8 __user *)(d); \
--
2.26.1
^ permalink raw reply related
* [PATCH v5 05/10] powerpc/signal64: Remove TM ifdefery in middle of if/else block
From: Christopher M. Riedl @ 2021-02-03 18:43 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20210203184323.20792-1-cmr@codefail.de>
Rework the messy ifdef breaking up the if-else for TM similar to
commit f1cf4f93de2f ("powerpc/signal32: Remove ifdefery in middle of if/else").
Unlike that commit for ppc32, the ifdef can't be removed entirely since
uc_transact in sigframe depends on CONFIG_PPC_TRANSACTIONAL_MEM.
Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
---
arch/powerpc/kernel/signal_64.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index b211a8ea4f6e..8e1d804ce552 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -710,9 +710,7 @@ SYSCALL_DEFINE0(rt_sigreturn)
struct pt_regs *regs = current_pt_regs();
struct ucontext __user *uc = (struct ucontext __user *)regs->gpr[1];
sigset_t set;
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
unsigned long msr;
-#endif
/* Always make any pending restarted system calls return -EINTR */
current->restart_block.fn = do_no_restart_syscall;
@@ -765,7 +763,10 @@ SYSCALL_DEFINE0(rt_sigreturn)
if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR]))
goto badframe;
+#endif
+
if (MSR_TM_ACTIVE(msr)) {
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* We recheckpoint on return. */
struct ucontext __user *uc_transact;
@@ -778,9 +779,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
if (restore_tm_sigcontexts(current, &uc->uc_mcontext,
&uc_transact->uc_mcontext))
goto badframe;
- } else
#endif
- {
+ } else {
/*
* Fall through, for non-TM restore
*
@@ -818,10 +818,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
unsigned long newsp = 0;
long err = 0;
struct pt_regs *regs = tsk->thread.regs;
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* Save the thread's msr before get_tm_stackpointer() changes it */
unsigned long msr = regs->msr;
-#endif
frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
if (!access_ok(frame, sizeof(*frame)))
@@ -836,8 +834,9 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
/* Create the ucontext. */
err |= __put_user(0, &frame->uc.uc_flags);
err |= __save_altstack(&frame->uc.uc_stack, regs->gpr[1]);
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+
if (MSR_TM_ACTIVE(msr)) {
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
/* The ucontext_t passed to userland points to the second
* ucontext_t (for transactional state) with its uc_link ptr.
*/
@@ -847,9 +846,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
tsk, ksig->sig, NULL,
(unsigned long)ksig->ka.sa.sa_handler,
msr);
- } else
#endif
- {
+ } else {
err |= __put_user(0, &frame->uc.uc_link);
prepare_setup_sigcontext(tsk, 1);
err |= setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
--
2.26.1
^ permalink raw reply related
* [PATCH v5 02/10] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user()
From: Christopher M. Riedl @ 2021-02-03 18:43 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20210203184323.20792-1-cmr@codefail.de>
Reuse the "safe" implementation from signal.c except for calling
unsafe_copy_from_user() to copy into a local buffer.
Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
---
arch/powerpc/kernel/signal.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index 2559a681536e..7dfc536c78ef 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -53,6 +53,30 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task, void __user *from);
&buf[i], label);\
} while (0)
+#define unsafe_copy_fpr_from_user(task, from, label) do { \
+ struct task_struct *__t = task; \
+ u64 __user *__f = (u64 __user *)from; \
+ u64 buf[ELF_NFPREG]; \
+ int i; \
+ \
+ unsafe_copy_from_user(buf, __f, sizeof(buf), label); \
+ for (i = 0; i < ELF_NFPREG - 1; i++) \
+ __t->thread.TS_FPR(i) = buf[i]; \
+ __t->thread.fp_state.fpscr = buf[i]; \
+} while (0)
+
+#define unsafe_copy_vsx_from_user(task, from, label) do { \
+ struct task_struct *__t = task; \
+ u64 __user *__f = (u64 __user *)from; \
+ u64 buf[ELF_NVSRHALFREG]; \
+ int i; \
+ \
+ unsafe_copy_from_user(buf, __f, sizeof(buf), label); \
+ for (i = 0; i < ELF_NVSRHALFREG ; i++) \
+ __t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i]; \
+} while (0)
+
+
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
#define unsafe_copy_ckfpr_to_user(to, task, label) do { \
struct task_struct *__t = task; \
@@ -80,6 +104,10 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task, void __user *from);
unsafe_copy_to_user(to, (task)->thread.fp_state.fpr, \
ELF_NFPREG * sizeof(double), label)
+#define unsafe_copy_fpr_from_user(task, from, label) \
+ unsafe_copy_from_user((task)->thread.fp_state.fpr, from, \
+ ELF_NFPREG * sizeof(double), label)
+
static inline unsigned long
copy_fpr_to_user(void __user *to, struct task_struct *task)
{
@@ -115,6 +143,8 @@ copy_ckfpr_from_user(struct task_struct *task, void __user *from)
#else
#define unsafe_copy_fpr_to_user(to, task, label) do { } while (0)
+#define unsafe_copy_fpr_from_user(task, from, label) do { } while (0)
+
static inline unsigned long
copy_fpr_to_user(void __user *to, struct task_struct *task)
{
--
2.26.1
^ permalink raw reply related
* Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
From: Lakshmi Ramasubramanian @ 2021-02-03 17:47 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: sashal, dmitry.kasatkin, linux-kernel, zohar, tyhicks, ebiederm,
gregkh, linux-integrity, linuxppc-dev
In-Reply-To: <87eeic1u6b.fsf@manicouagan.localdomain>
On 1/22/21 2:30 PM, Thiago Jung Bauermann wrote:
>
> Hi Lakshmi,
>
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>
>> IMA allocates kernel virtual memory to carry forward the measurement
>> list, from the current kernel to the next kernel on kexec system call,
>> in ima_add_kexec_buffer() function. In error code paths this memory
>> is not freed resulting in memory leak.
>>
>> Free the memory allocated for the IMA measurement list in
>> the error code paths in ima_add_kexec_buffer() function.
>>
>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com>
>> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
>> ---
>> security/integrity/ima/ima_kexec.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index 121de3e04af2..212145008a01 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -119,12 +119,14 @@ void ima_add_kexec_buffer(struct kimage *image)
>> ret = kexec_add_buffer(&kbuf);
>> if (ret) {
>> pr_err("Error passing over kexec measurement buffer.\n");
>> + vfree(kexec_buffer);
>> return;
>> }
>
> This is a good catch.
Thanks.
>
>>
>> ret = arch_ima_add_kexec_buffer(image, kbuf.mem, kexec_segment_size);
>> if (ret) {
>> pr_err("Error passing over kexec measurement buffer.\n");
>> + vfree(kexec_buffer);
>> return;
>> }
>
> But this would cause problems, because the buffer is still there in the
> kimage and would cause kimage_load_segment() to access invalid memory.
>
> There's no function to undo a kexec_add_buffer() to avoid this problem,
> so I'd suggest just accepting the leak in this case. Fortunately, the
> current implementations of arch_ima_add_kexec_buffer() are very simple
> and cannot fail, so this is a theoretical problem.
>
Agreed. I'll post a new patch with the above change removed.
thanks,
-lakshmi
^ permalink raw reply
* Re: [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
From: Lakshmi Ramasubramanian @ 2021-02-03 17:46 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: sashal, dmitry.kasatkin, linux-kernel, zohar, tyhicks, ebiederm,
gregkh, linux-integrity, linuxppc-dev
In-Reply-To: <87bldg1u3s.fsf@manicouagan.localdomain>
On 1/22/21 2:31 PM, Thiago Jung Bauermann wrote:
>
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>
>> IMA allocates kernel virtual memory to carry forward the measurement
>> list, from the current kernel to the next kernel on kexec system call,
>> in ima_add_kexec_buffer() function. This buffer is not freed before
>> completing the kexec system call resulting in memory leak.
>>
>> Add ima_buffer field in "struct kimage" to store the virtual address
>> of the buffer allocated for the IMA measurement list.
>> Free the memory allocated for the IMA measurement list in
>> kimage_file_post_load_cleanup() function.
>>
>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com>
>> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
>
> Good catch.
>
> Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
Thanks Thiago.
-lakshmi
^ permalink raw reply
* Re: [PATCH v3 28/32] powerpc/64s: interrupt implement exit logic in C
From: Christophe Leroy @ 2021-02-03 16:25 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Michal Suchanek
In-Reply-To: <20200225173541.1549955-29-npiggin@gmail.com>
Le 25/02/2020 à 18:35, Nicholas Piggin a écrit :
> Implement the bulk of interrupt return logic in C. The asm return code
> must handle a few cases: restoring full GPRs, and emulating stack store.
>
> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
> +{
> + unsigned long *ti_flagsp = ¤t_thread_info()->flags;
> + unsigned long flags;
> +
> + if (IS_ENABLED(CONFIG_PPC_BOOK3S) && unlikely(!(regs->msr & MSR_RI)))
> + unrecoverable_exception(regs);
> + BUG_ON(regs->msr & MSR_PR);
> + BUG_ON(!FULL_REGS(regs));
> +
> + local_irq_save(flags);
> +
> + if (regs->softe == IRQS_ENABLED) {
> + /* Returning to a kernel context with local irqs enabled. */
> + WARN_ON_ONCE(!(regs->msr & MSR_EE));
> +again:
> + if (IS_ENABLED(CONFIG_PREEMPT)) {
> + /* Return to preemptible kernel context */
> + if (unlikely(*ti_flagsp & _TIF_NEED_RESCHED)) {
> + if (preempt_count() == 0)
> + preempt_schedule_irq();
> + }
> + }
> +
> + trace_hardirqs_on();
> + __hard_EE_RI_disable();
> + if (unlikely(lazy_irq_pending())) {
> + __hard_RI_enable();
> + irq_soft_mask_set(IRQS_ALL_DISABLED);
> + trace_hardirqs_off();
> + local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
> + /*
> + * Can't local_irq_enable in case we are in interrupt
> + * context. Must replay directly.
> + */
> + replay_soft_interrupts();
> + irq_soft_mask_set(flags);
> + /* Took an interrupt, may have more exit work to do. */
> + goto again;
> + }
> + local_paca->irq_happened = 0;
> + irq_soft_mask_set(IRQS_ENABLED);
> + } else {
> + /* Returning to a kernel context with local irqs disabled. */
> + trace_hardirqs_on();
> + __hard_EE_RI_disable();
> + if (regs->msr & MSR_EE)
> + local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
> + }
> +
> +
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> + local_paca->tm_scratch = regs->msr;
> +#endif
> +
> + /*
> + * We don't need to restore AMR on the way back to userspace for KUAP.
> + * The value of AMR only matters while we're in the kernel.
> + */
> + kuap_restore_amr(regs);
Is that correct to restore KUAP state here ? Shouldn't we have it at lower level in assembly ?
Isn't there a risk that someone manages to call interrupt_exit_kernel_prepare() or the end of it in
a way or another, and get the previous KUAP state restored by this way ?
Also, it looks a bit strange to have kuap_save_amr_and_lock() done at lowest level in assembly, and
kuap_restore_amr() done in upper level. That looks unbalanced.
Christophe
> +
> + if (unlikely(*ti_flagsp & _TIF_EMULATE_STACK_STORE)) {
> + clear_bits(_TIF_EMULATE_STACK_STORE, ti_flagsp);
> + return 1;
> + }
> + return 0;
> +}
> +#endif
> diff --git a/arch/powerpc/kernel/vector.S b/arch/powerpc/kernel/vector.S
> index 25c14a0981bf..d20c5e79e03c 100644
> --- a/arch/powerpc/kernel/vector.S
> +++ b/arch/powerpc/kernel/vector.S
> @@ -134,7 +134,7 @@ _GLOBAL(load_up_vsx)
> /* enable use of VSX after return */
> oris r12,r12,MSR_VSX@h
> std r12,_MSR(r1)
> - b fast_exception_return
> + b fast_interrupt_return
>
> #endif /* CONFIG_VSX */
>
>
^ permalink raw reply
* Re: [PATCH 3/3] tools/perf: Add perf tools support to expose Performance Monitor Counter SPRs as part of extended regs
From: Arnaldo Carvalho de Melo @ 2021-02-03 16:25 UTC (permalink / raw)
To: Athira Rajeev; +Cc: maddy, linuxppc-dev, jolsa, kjain
In-Reply-To: <1612335337-1888-4-git-send-email-atrajeev@linux.vnet.ibm.com>
Em Wed, Feb 03, 2021 at 01:55:37AM -0500, Athira Rajeev escreveu:
> To enable presenting of Performance Monitor Counter Registers
> (PMC1 to PMC6) as part of extended regsiters, patch adds these
> to sample_reg_mask in the tool side (to use with -I? option).
>
> Simplified the PERF_REG_PMU_MASK_300/31 definition. Excluded the
> unsupported SPRs (MMCR3, SIER2, SIER3) from extended mask value for
> CPU_FTR_ARCH_300.
Applied just 3/3, the tooling part, to my local branch, please holler if
I should wait a bit more.
- Arnaldo
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> tools/arch/powerpc/include/uapi/asm/perf_regs.h | 28 +++++++++++++++++++------
> tools/perf/arch/powerpc/include/perf_regs.h | 6 ++++++
> tools/perf/arch/powerpc/util/perf_regs.c | 6 ++++++
> 3 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> index bdf5f10f8b9f..578b3ee86105 100644
> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -55,17 +55,33 @@ enum perf_event_powerpc_regs {
> PERF_REG_POWERPC_MMCR3,
> PERF_REG_POWERPC_SIER2,
> PERF_REG_POWERPC_SIER3,
> + PERF_REG_POWERPC_PMC1,
> + PERF_REG_POWERPC_PMC2,
> + PERF_REG_POWERPC_PMC3,
> + PERF_REG_POWERPC_PMC4,
> + PERF_REG_POWERPC_PMC5,
> + PERF_REG_POWERPC_PMC6,
> /* Max regs without the extended regs */
> PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
> };
>
> #define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)
>
> -/* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300 */
> -#define PERF_REG_PMU_MASK_300 (((1ULL << (PERF_REG_POWERPC_MMCR2 + 1)) - 1) - PERF_REG_PMU_MASK)
> -/* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31 */
> -#define PERF_REG_PMU_MASK_31 (((1ULL << (PERF_REG_POWERPC_SIER3 + 1)) - 1) - PERF_REG_PMU_MASK)
> +/* Exclude MMCR3, SIER2, SIER3 for CPU_FTR_ARCH_300 */
> +#define PERF_EXCLUDE_REG_EXT_300 (7ULL << PERF_REG_POWERPC_MMCR3)
>
> -#define PERF_REG_MAX_ISA_300 (PERF_REG_POWERPC_MMCR2 + 1)
> -#define PERF_REG_MAX_ISA_31 (PERF_REG_POWERPC_SIER3 + 1)
> +/*
> + * PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300
> + * includes 9 SPRS from MMCR0 to PMC6 excluding the
> + * unsupported SPRS in PERF_EXCLUDE_REG_EXT_300.
> + */
> +#define PERF_REG_PMU_MASK_300 ((0xfffULL << PERF_REG_POWERPC_MMCR0) - PERF_EXCLUDE_REG_EXT_300)
> +
> +/*
> + * PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31
> + * includes 12 SPRs from MMCR0 to PMC6.
> + */
> +#define PERF_REG_PMU_MASK_31 (0xfffULL << PERF_REG_POWERPC_MMCR0)
> +
> +#define PERF_REG_EXTENDED_MAX (PERF_REG_POWERPC_PMC6 + 1)
> #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
> index 63f3ac91049f..98b6f9eabfc3 100644
> --- a/tools/perf/arch/powerpc/include/perf_regs.h
> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
> @@ -71,6 +71,12 @@
> [PERF_REG_POWERPC_MMCR3] = "mmcr3",
> [PERF_REG_POWERPC_SIER2] = "sier2",
> [PERF_REG_POWERPC_SIER3] = "sier3",
> + [PERF_REG_POWERPC_PMC1] = "pmc1",
> + [PERF_REG_POWERPC_PMC2] = "pmc2",
> + [PERF_REG_POWERPC_PMC3] = "pmc3",
> + [PERF_REG_POWERPC_PMC4] = "pmc4",
> + [PERF_REG_POWERPC_PMC5] = "pmc5",
> + [PERF_REG_POWERPC_PMC6] = "pmc6",
> };
>
> static inline const char *perf_reg_name(int id)
> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
> index 2b6d4704e3aa..8116a253f91f 100644
> --- a/tools/perf/arch/powerpc/util/perf_regs.c
> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
> @@ -68,6 +68,12 @@
> SMPL_REG(mmcr3, PERF_REG_POWERPC_MMCR3),
> SMPL_REG(sier2, PERF_REG_POWERPC_SIER2),
> SMPL_REG(sier3, PERF_REG_POWERPC_SIER3),
> + SMPL_REG(pmc1, PERF_REG_POWERPC_PMC1),
> + SMPL_REG(pmc2, PERF_REG_POWERPC_PMC2),
> + SMPL_REG(pmc3, PERF_REG_POWERPC_PMC3),
> + SMPL_REG(pmc4, PERF_REG_POWERPC_PMC4),
> + SMPL_REG(pmc5, PERF_REG_POWERPC_PMC5),
> + SMPL_REG(pmc6, PERF_REG_POWERPC_PMC6),
> SMPL_REG_END
> };
>
> --
> 1.8.3.1
>
--
- Arnaldo
^ permalink raw reply
* Re: [PATCH] tools/perf: Fix powerpc gap between kernel end and module start
From: Arnaldo Carvalho de Melo @ 2021-02-03 15:31 UTC (permalink / raw)
To: Athira Rajeev
Cc: linuxppc-dev, Madhavan Srinivasan, Jiri Olsa, Jiri Olsa,
Kajol Jain
In-Reply-To: <1D564116-5FAC-4775-BEB2-4D5B837080D6@linux.vnet.ibm.com>
Em Tue, Feb 02, 2021 at 04:02:36PM +0530, Athira Rajeev escreveu:
>
>
> On 18-Jan-2021, at 3:51 PM, kajoljain <kjain@linux.ibm.com> wrote:
>
>
>
> On 1/12/21 3:08 PM, Jiri Olsa wrote:
>
> On Mon, Dec 28, 2020 at 09:14:14PM -0500, Athira Rajeev wrote:
>
> SNIP
>
>
> c000000002799370 b backtrace_flag
> c000000002799378 B radix_tree_node_cachep
> c000000002799380 B __bss_stop
> c0000000027a0000 B _end
> c008000003890000 t icmp_checkentry [ip_tables]
> c008000003890038 t ipt_alloc_initial_table [ip_tables]
> c008000003890468 T ipt_do_table [ip_tables]
> c008000003890de8 T ipt_unregister_table_pre_exit [ip_tables]
> ...
>
> Perf calls function symbols__fixup_end() which sets the end of
> symbol
> to 0xc008000003890000, which is the next address and this is the
> start
> address of first module (icmp_checkentry in above) which will make
> the
> huge symbol size of 0x80000010f0000.
>
> After symbols__fixup_end:
> symbols__fixup_end: sym->name: _end, sym->start:
> 0xc0000000027a0000,
> sym->end: 0xc008000003890000
>
> On powerpc, kernel text segment is located at 0xc000000000000000
> whereas the modules are located at very high memory addresses,
> 0xc00800000xxxxxxx. Since the gap between end of kernel text
> segment
> and beginning of first module's address is high, histogram
> allocation
> using calloc fails.
>
> Fix this by detecting the kernel's last symbol and limiting
> the range of last kernel symbol to pagesize.
>
>
> Patch looks good to me.
>
> Tested-By: Kajol Jain<kjain@linux.ibm.com>
>
> Thanks,
> Kajol Jain
>
>
> Signed-off-by: Athira Rajeev<atrajeev@linux.vnet.ibm.com>
>
>
> I can't test, but since the same approach works for arm and s390,
> this also looks ok
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
>
> thanks,
> jirka
>
>
> Hi Arnaldo,
>
> Can you please help review this patch and merge if this looks good..
Thanks, collected the Tested-by from Kajol and the Acked-by from Jiri
and applied to my local tree for testing, then up to my perf/core
branch.
- Arnaldo
^ permalink raw reply
* Re: [PATCH v10 6/6] powerpc: Book3S 64-bit outline-only KASAN support
From: Daniel Axtens @ 2021-02-03 12:53 UTC (permalink / raw)
To: Christophe Leroy, linux-kernel, linux-mm, linuxppc-dev, kasan-dev,
aneesh.kumar, bsingharora
In-Reply-To: <4b790789-052f-76de-a289-726517026efd@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 03/02/2021 à 12:59, Daniel Axtens a écrit :
>> Implement a limited form of KASAN for Book3S 64-bit machines running under
>> the Radix MMU, supporting only outline mode.
>>
>
>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> index a66f435dabbf..9a6fd603f0e7 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -2157,8 +2157,8 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
>> break;
>>
>> stack = (unsigned long *) sp;
>> - newsp = stack[0];
>> - ip = stack[STACK_FRAME_LR_SAVE];
>> + newsp = READ_ONCE_NOCHECK(stack[0]);
>> + ip = READ_ONCE_NOCHECK(stack[STACK_FRAME_LR_SAVE]);
>> if (!firstframe || ip != lr) {
>> printk("%s["REG"] ["REG"] %pS",
>> loglvl, sp, ip, (void *)ip);
>> @@ -2176,17 +2176,19 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
>> * See if this is an exception frame.
>> * We look for the "regshere" marker in the current frame.
>> */
>> - if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE)
>> - && stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
>> + if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE) &&
>> + (READ_ONCE_NOCHECK(stack[STACK_FRAME_MARKER]) ==
>> + STACK_FRAME_REGS_MARKER)) {
>> struct pt_regs *regs = (struct pt_regs *)
>> (sp + STACK_FRAME_OVERHEAD);
>>
>> - lr = regs->link;
>> + lr = READ_ONCE_NOCHECK(regs->link);
>> printk("%s--- interrupt: %lx at %pS\n",
>> - loglvl, regs->trap, (void *)regs->nip);
>> + loglvl, READ_ONCE_NOCHECK(regs->trap),
>> + (void *)READ_ONCE_NOCHECK(regs->nip));
>> __show_regs(regs);
>> printk("%s--- interrupt: %lx\n",
>> - loglvl, regs->trap);
>> + loglvl, READ_ONCE_NOCHECK(regs->trap));
>>
>> firstframe = 1;
>> }
>
>
> The above changes look like a bug fix not directly related to KASAN. Should be split out in another
> patch I think.
That code corresponds to the following part of the patch description:
| - Make our stack-walking code KASAN-safe by using READ_ONCE_NOCHECK -
| generic code, arm64, s390 and x86 all do this for similar sorts of
| reasons: when unwinding a stack, we might touch memory that KASAN has
| marked as being out-of-bounds. In our case we often get this when
| checking for an exception frame because we're checking an arbitrary
| offset into the stack frame.
|
| See commit 20955746320e ("s390/kasan: avoid false positives during stack
| unwind"), commit bcaf669b4bdb ("arm64: disable kasan when accessing
| frame->fp in unwind_frame"), commit 91e08ab0c851 ("x86/dumpstack:
| Prevent KASAN false positive warnings") and commit 6e22c8366416
| ("tracing, kasan: Silence Kasan warning in check_stack of stack_tracer")
include/linux/compiler.h describes it as follows:
/*
* Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
* to hide memory access from KASAN.
*/
So I think it is sufficently connected with KASAN to be in this patch.
Kind regards,
Daniel
>
> Christophe
^ permalink raw reply
* Re: [PATCH v10 6/6] powerpc: Book3S 64-bit outline-only KASAN support
From: Christophe Leroy @ 2021-02-03 12:45 UTC (permalink / raw)
To: Daniel Axtens, linux-kernel, linux-mm, linuxppc-dev, kasan-dev,
aneesh.kumar, bsingharora
In-Reply-To: <20210203115946.663273-7-dja@axtens.net>
Le 03/02/2021 à 12:59, Daniel Axtens a écrit :
> Implement a limited form of KASAN for Book3S 64-bit machines running under
> the Radix MMU, supporting only outline mode.
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index a66f435dabbf..9a6fd603f0e7 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -2157,8 +2157,8 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
> break;
>
> stack = (unsigned long *) sp;
> - newsp = stack[0];
> - ip = stack[STACK_FRAME_LR_SAVE];
> + newsp = READ_ONCE_NOCHECK(stack[0]);
> + ip = READ_ONCE_NOCHECK(stack[STACK_FRAME_LR_SAVE]);
> if (!firstframe || ip != lr) {
> printk("%s["REG"] ["REG"] %pS",
> loglvl, sp, ip, (void *)ip);
> @@ -2176,17 +2176,19 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
> * See if this is an exception frame.
> * We look for the "regshere" marker in the current frame.
> */
> - if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE)
> - && stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
> + if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE) &&
> + (READ_ONCE_NOCHECK(stack[STACK_FRAME_MARKER]) ==
> + STACK_FRAME_REGS_MARKER)) {
> struct pt_regs *regs = (struct pt_regs *)
> (sp + STACK_FRAME_OVERHEAD);
>
> - lr = regs->link;
> + lr = READ_ONCE_NOCHECK(regs->link);
> printk("%s--- interrupt: %lx at %pS\n",
> - loglvl, regs->trap, (void *)regs->nip);
> + loglvl, READ_ONCE_NOCHECK(regs->trap),
> + (void *)READ_ONCE_NOCHECK(regs->nip));
> __show_regs(regs);
> printk("%s--- interrupt: %lx\n",
> - loglvl, regs->trap);
> + loglvl, READ_ONCE_NOCHECK(regs->trap));
>
> firstframe = 1;
> }
The above changes look like a bug fix not directly related to KASAN. Should be split out in another
patch I think.
Christophe
^ permalink raw reply
* [PATCH v10 6/6] powerpc: Book3S 64-bit outline-only KASAN support
From: Daniel Axtens @ 2021-02-03 11:59 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: Daniel Axtens
In-Reply-To: <20210203115946.663273-1-dja@axtens.net>
Implement a limited form of KASAN for Book3S 64-bit machines running under
the Radix MMU, supporting only outline mode.
- Enable the compiler instrumentation to check addresses and maintain the
shadow region. (This is the guts of KASAN which we can easily reuse.)
- Require kasan-vmalloc support to handle modules and anything else in
vmalloc space.
- KASAN needs to be able to validate all pointer accesses, but we can't
instrument all kernel addresses - only linear map and vmalloc. On boot,
set up a single page of read-only shadow that marks all iomap and
vmemmap accesses as valid.
- Make our stack-walking code KASAN-safe by using READ_ONCE_NOCHECK -
generic code, arm64, s390 and x86 all do this for similar sorts of
reasons: when unwinding a stack, we might touch memory that KASAN has
marked as being out-of-bounds. In our case we often get this when
checking for an exception frame because we're checking an arbitrary
offset into the stack frame.
See commit 20955746320e ("s390/kasan: avoid false positives during stack
unwind"), commit bcaf669b4bdb ("arm64: disable kasan when accessing
frame->fp in unwind_frame"), commit 91e08ab0c851 ("x86/dumpstack:
Prevent KASAN false positive warnings") and commit 6e22c8366416
("tracing, kasan: Silence Kasan warning in check_stack of stack_tracer")
- Document KASAN in both generic and powerpc docs.
Background
----------
KASAN support on Book3S is a bit tricky to get right:
- It would be good to support inline instrumentation so as to be able to
catch stack issues that cannot be caught with outline mode.
- Inline instrumentation requires a fixed offset.
- Book3S runs code with translations off ("real mode") during boot,
including a lot of generic device-tree parsing code which is used to
determine MMU features.
[ppc64 mm note: The kernel installs a linear mapping at effective
address c000...-c008.... This is a one-to-one mapping with physical
memory from 0000... onward. Because of how memory accesses work on
powerpc 64-bit Book3S, a kernel pointer in the linear map accesses the
same memory both with translations on (accessing as an 'effective
address'), and with translations off (accessing as a 'real
address'). This works in both guests and the hypervisor. For more
details, see s5.7 of Book III of version 3 of the ISA, in particular
the Storage Control Overview, s5.7.3, and s5.7.5 - noting that this
KASAN implementation currently only supports Radix.]
- Some code - most notably a lot of KVM code - also runs with translations
off after boot.
- Therefore any offset has to point to memory that is valid with
translations on or off.
One approach is just to give up on inline instrumentation. This way
boot-time checks can be delayed until after the MMU is set is up, and we
can just not instrument any code that runs with translations off after
booting. Take this approach for now and require outline instrumentation.
Previous attempts allowed inline instrumentation. However, they came with
some unfortunate restrictions: only physically contiguous memory could be
used and it had to be specified at compile time. Maybe we can do better in
the future.
Cc: Balbir Singh <bsingharora@gmail.com> # ppc64 out-of-line radix version
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> # ppc64 hash version
Cc: Christophe Leroy <christophe.leroy@csgroup.eu> # ppc32 version
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
Documentation/dev-tools/kasan.rst | 9 +-
Documentation/powerpc/kasan.txt | 48 +++++++++-
arch/powerpc/Kconfig | 4 +-
arch/powerpc/Kconfig.debug | 3 +-
arch/powerpc/include/asm/book3s/64/hash.h | 4 +
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +
arch/powerpc/include/asm/book3s/64/radix.h | 13 ++-
arch/powerpc/include/asm/kasan.h | 22 +++++
arch/powerpc/kernel/Makefile | 11 +++
arch/powerpc/kernel/process.c | 16 ++--
arch/powerpc/kvm/Makefile | 5 ++
arch/powerpc/mm/book3s64/Makefile | 9 ++
arch/powerpc/mm/kasan/Makefile | 1 +
arch/powerpc/mm/kasan/init_book3s_64.c | 95 ++++++++++++++++++++
arch/powerpc/mm/ptdump/ptdump.c | 20 ++++-
arch/powerpc/platforms/Kconfig.cputype | 1 +
arch/powerpc/platforms/powernv/Makefile | 6 ++
arch/powerpc/platforms/pseries/Makefile | 3 +
18 files changed, 256 insertions(+), 18 deletions(-)
create mode 100644 arch/powerpc/mm/kasan/init_book3s_64.c
diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index 9cfc116cc6bf..8c5d0c5c35f6 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -22,8 +22,9 @@ out-of-bounds accesses for global variables is only supported since Clang 11.
Tag-based KASAN is only supported in Clang.
Currently generic KASAN is supported for the x86_64, arm, arm64, xtensa, s390
-and riscv architectures. It is also supported on 32-bit powerpc kernels.
-Tag-based KASAN modes are supported only for arm64.
+and riscv architectures. It is also supported on powerpc for 32-bit powerpc
+kernels and for 64-bit kernels running under the Radix MMU. Tag-based KASAN
+modes are supported only for arm64.
Usage
-----
@@ -334,8 +335,8 @@ CONFIG_KASAN_VMALLOC
With ``CONFIG_KASAN_VMALLOC``, KASAN can cover vmalloc space at the
cost of greater memory usage. Currently this supported on x86, s390
-and 32-bit powerpc. It is optional, except on 32-bit powerpc kernels
-with module support, where it is required.
+and powerpc. It is optional, except on 64-bit powerpc kernels, and on
+32-bit powerpc kernels with module support, where it is required.
This works by hooking into vmalloc and vmap, and dynamically
allocating real shadow memory to back the mappings.
diff --git a/Documentation/powerpc/kasan.txt b/Documentation/powerpc/kasan.txt
index 26bb0e8bb18c..f032b4eaf205 100644
--- a/Documentation/powerpc/kasan.txt
+++ b/Documentation/powerpc/kasan.txt
@@ -1,4 +1,4 @@
-KASAN is supported on powerpc on 32-bit only.
+KASAN is supported on powerpc on 32-bit and Radix 64-bit only.
32 bit support
==============
@@ -10,3 +10,49 @@ fixmap area and occupies one eighth of the total kernel virtual memory space.
Instrumentation of the vmalloc area is optional, unless built with modules,
in which case it is required.
+
+64 bit support
+==============
+
+Currently, only the radix MMU is supported. There have been versions for hash
+and Book3E processors floating around on the mailing list, but nothing has been
+merged.
+
+KASAN support on Book3S is a bit tricky to get right:
+
+ - It would be good to support inline instrumentation so as to be able to catch
+ stack issues that cannot be caught with outline mode.
+
+ - Inline instrumentation requires a fixed offset.
+
+ - Book3S runs code with translations off ("real mode") during boot, including a
+ lot of generic device-tree parsing code which is used to determine MMU
+ features.
+
+ - Some code - most notably a lot of KVM code - also runs with translations off
+ after boot.
+
+ - Therefore any offset has to point to memory that is valid with
+ translations on or off.
+
+One approach is just to give up on inline instrumentation. This way boot-time
+checks can be delayed until after the MMU is set is up, and we can just not
+instrument any code that runs with translations off after booting. This is the
+current approach.
+
+To avoid this limitiation, the KASAN shadow would have to be placed inside the
+linear mapping, using the same high-bits trick we use for the rest of the linear
+mapping. This is tricky:
+
+ - We'd like to place it near the start of physical memory. In theory we can do
+ this at run-time based on how much physical memory we have, but this requires
+ being able to arbitrarily relocate the kernel, which is basically the tricky
+ part of KASLR. Not being game to implement both tricky things at once, this
+ is hopefully something we can revisit once we get KASLR for Book3S.
+
+ - Alternatively, we can place the shadow at the _end_ of memory, but this
+ requires knowing how much contiguous physical memory a system has _at compile
+ time_. This is a big hammer, and has some unfortunate consequences: inablity
+ to handle discontiguous physical memory, total failure to boot on machines
+ with less memory than specified, and that machines with more memory than
+ specified can't use it. This was deemed unacceptable.
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9141f03060ce..a1f359ec2fa1 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -118,6 +118,7 @@ config PPC
# Please keep this list sorted alphabetically.
#
select ARCH_32BIT_OFF_T if PPC32
+ select ARCH_DISABLE_KASAN_INLINE if PPC_RADIX_MMU
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEVMEM_IS_ALLOWED
select ARCH_HAS_ELF_RANDOMIZE
@@ -183,7 +184,8 @@ config PPC
select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
- select HAVE_ARCH_KASAN_VMALLOC if PPC32 && PPC_PAGE_SHIFT <= 14
+ select HAVE_ARCH_KASAN if PPC_RADIX_MMU
+ select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN
select HAVE_ARCH_KGDB
select HAVE_ARCH_MMAP_RND_BITS
select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index b88900f4832f..33e4cc8c4306 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -397,4 +397,5 @@ config PPC_FAST_ENDIAN_SWITCH
config KASAN_SHADOW_OFFSET
hex
depends on KASAN
- default 0xe0000000
+ default 0xe0000000 if PPC32
+ default 0xa80e000000000000 if PPC64
diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
index d959b0195ad9..222669864ff6 100644
--- a/arch/powerpc/include/asm/book3s/64/hash.h
+++ b/arch/powerpc/include/asm/book3s/64/hash.h
@@ -18,6 +18,10 @@
#include <asm/book3s/64/hash-4k.h>
#endif
+#define H_PTRS_PER_PTE (1 << H_PTE_INDEX_SIZE)
+#define H_PTRS_PER_PMD (1 << H_PMD_INDEX_SIZE)
+#define H_PTRS_PER_PUD (1 << H_PUD_INDEX_SIZE)
+
/* Bits to set in a PMD/PUD/PGD entry valid bit*/
#define HASH_PMD_VAL_BITS (0x8000000000000000UL)
#define HASH_PUD_VAL_BITS (0x8000000000000000UL)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index a39886681629..891084efdad7 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -230,6 +230,10 @@ extern unsigned long __pmd_frag_size_shift;
#define PTRS_PER_PUD (1 << PUD_INDEX_SIZE)
#define PTRS_PER_PGD (1 << PGD_INDEX_SIZE)
+#define MAX_PTRS_PER_PTE ((H_PTRS_PER_PTE > R_PTRS_PER_PTE) ? H_PTRS_PER_PTE : R_PTRS_PER_PTE)
+#define MAX_PTRS_PER_PMD ((H_PTRS_PER_PMD > R_PTRS_PER_PMD) ? H_PTRS_PER_PMD : R_PTRS_PER_PMD)
+#define MAX_PTRS_PER_PUD ((H_PTRS_PER_PUD > R_PTRS_PER_PUD) ? H_PTRS_PER_PUD : R_PTRS_PER_PUD)
+
/* PMD_SHIFT determines what a second-level page table entry can map */
#define PMD_SHIFT (PAGE_SHIFT + PTE_INDEX_SIZE)
#define PMD_SIZE (1UL << PMD_SHIFT)
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h b/arch/powerpc/include/asm/book3s/64/radix.h
index c7813dc628fc..b3492b80f858 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -35,6 +35,11 @@
#define RADIX_PMD_SHIFT (PAGE_SHIFT + RADIX_PTE_INDEX_SIZE)
#define RADIX_PUD_SHIFT (RADIX_PMD_SHIFT + RADIX_PMD_INDEX_SIZE)
#define RADIX_PGD_SHIFT (RADIX_PUD_SHIFT + RADIX_PUD_INDEX_SIZE)
+
+#define R_PTRS_PER_PTE (1 << RADIX_PTE_INDEX_SIZE)
+#define R_PTRS_PER_PMD (1 << RADIX_PMD_INDEX_SIZE)
+#define R_PTRS_PER_PUD (1 << RADIX_PUD_INDEX_SIZE)
+
/*
* Size of EA range mapped by our pagetables.
*/
@@ -68,11 +73,11 @@
*
*
* 3rd quadrant expanded:
- * +------------------------------+
+ * +------------------------------+ Highest address (0xc010000000000000)
+ * +------------------------------+ KASAN shadow end (0xc00fc00000000000)
* | |
* | |
- * | |
- * +------------------------------+ Kernel vmemmap end (0xc010000000000000)
+ * +------------------------------+ Kernel vmemmap end/shadow start (0xc00e000000000000)
* | |
* | 512TB |
* | |
@@ -126,6 +131,8 @@
#define RADIX_VMEMMAP_SIZE RADIX_KERN_MAP_SIZE
#define RADIX_VMEMMAP_END (RADIX_VMEMMAP_START + RADIX_VMEMMAP_SIZE)
+/* For the sizes of the shadow area, see kasan.h */
+
#ifndef __ASSEMBLY__
#define RADIX_PTE_TABLE_SIZE (sizeof(pte_t) << RADIX_PTE_INDEX_SIZE)
#define RADIX_PMD_TABLE_SIZE (sizeof(pmd_t) << RADIX_PMD_INDEX_SIZE)
diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
index 7355ed05e65e..fdcce89c2515 100644
--- a/arch/powerpc/include/asm/kasan.h
+++ b/arch/powerpc/include/asm/kasan.h
@@ -30,7 +30,29 @@
#define KASAN_SHADOW_OFFSET ASM_CONST(CONFIG_KASAN_SHADOW_OFFSET)
+#ifdef CONFIG_PPC32
#define KASAN_SHADOW_END (-(-KASAN_SHADOW_START >> KASAN_SHADOW_SCALE_SHIFT))
+#endif
+
+#ifdef CONFIG_PPC_BOOK3S_64
+/*
+ * The shadow ends before the highest accessible address
+ * because we don't need a shadow for the shadow. Instead:
+ * c00e000000000000 << 3 + a80e000000000000000 = c00fc00000000000
+ */
+#define KASAN_SHADOW_END 0xc00fc00000000000UL
+
+DECLARE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
+
+static __always_inline bool kasan_arch_is_ready(void)
+{
+ if (static_branch_likely(&powerpc_kasan_enabled_key))
+ return true;
+ return false;
+}
+
+#define kasan_arch_is_ready kasan_arch_is_ready
+#endif
#ifdef CONFIG_KASAN
void kasan_early_init(void);
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index fe2ef598e2ea..a0cbe8953238 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -32,6 +32,17 @@ KASAN_SANITIZE_early_32.o := n
KASAN_SANITIZE_cputable.o := n
KASAN_SANITIZE_prom_init.o := n
KASAN_SANITIZE_btext.o := n
+KASAN_SANITIZE_paca.o := n
+KASAN_SANITIZE_setup_64.o := n
+KASAN_SANITIZE_mce.o := n
+KASAN_SANITIZE_mce_power.o := n
+
+# we have to be particularly careful in ppc64 to exclude code that
+# runs with translations off, as we cannot access the shadow with
+# translations off. However, ppc32 can sanitize this.
+ifdef CONFIG_PPC64
+KASAN_SANITIZE_traps.o := n
+endif
ifdef CONFIG_KASAN
CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index a66f435dabbf..9a6fd603f0e7 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2157,8 +2157,8 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
break;
stack = (unsigned long *) sp;
- newsp = stack[0];
- ip = stack[STACK_FRAME_LR_SAVE];
+ newsp = READ_ONCE_NOCHECK(stack[0]);
+ ip = READ_ONCE_NOCHECK(stack[STACK_FRAME_LR_SAVE]);
if (!firstframe || ip != lr) {
printk("%s["REG"] ["REG"] %pS",
loglvl, sp, ip, (void *)ip);
@@ -2176,17 +2176,19 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
* See if this is an exception frame.
* We look for the "regshere" marker in the current frame.
*/
- if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE)
- && stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
+ if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE) &&
+ (READ_ONCE_NOCHECK(stack[STACK_FRAME_MARKER]) ==
+ STACK_FRAME_REGS_MARKER)) {
struct pt_regs *regs = (struct pt_regs *)
(sp + STACK_FRAME_OVERHEAD);
- lr = regs->link;
+ lr = READ_ONCE_NOCHECK(regs->link);
printk("%s--- interrupt: %lx at %pS\n",
- loglvl, regs->trap, (void *)regs->nip);
+ loglvl, READ_ONCE_NOCHECK(regs->trap),
+ (void *)READ_ONCE_NOCHECK(regs->nip));
__show_regs(regs);
printk("%s--- interrupt: %lx\n",
- loglvl, regs->trap);
+ loglvl, READ_ONCE_NOCHECK(regs->trap));
firstframe = 1;
}
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 2bfeaa13befb..7f1592dacbeb 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -136,3 +136,8 @@ obj-$(CONFIG_KVM_BOOK3S_64_PR) += kvm-pr.o
obj-$(CONFIG_KVM_BOOK3S_64_HV) += kvm-hv.o
obj-y += $(kvm-book3s_64-builtin-objs-y)
+
+# KVM does a lot in real-mode, and 64-bit Book3S KASAN doesn't support that
+ifdef CONFIG_PPC_BOOK3S_64
+KASAN_SANITIZE := n
+endif
diff --git a/arch/powerpc/mm/book3s64/Makefile b/arch/powerpc/mm/book3s64/Makefile
index 1b56d3af47d4..a7d8a68bd2c5 100644
--- a/arch/powerpc/mm/book3s64/Makefile
+++ b/arch/powerpc/mm/book3s64/Makefile
@@ -21,3 +21,12 @@ obj-$(CONFIG_PPC_PKEY) += pkeys.o
# Instrumenting the SLB fault path can lead to duplicate SLB entries
KCOV_INSTRUMENT_slb.o := n
+
+# Parts of these can run in real mode and therefore are
+# not safe with the current outline KASAN implementation
+KASAN_SANITIZE_mmu_context.o := n
+KASAN_SANITIZE_pgtable.o := n
+KASAN_SANITIZE_radix_pgtable.o := n
+KASAN_SANITIZE_radix_tlb.o := n
+KASAN_SANITIZE_slb.o := n
+KASAN_SANITIZE_pkeys.o := n
diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
index 42fb628a44fd..07eef87abd6c 100644
--- a/arch/powerpc/mm/kasan/Makefile
+++ b/arch/powerpc/mm/kasan/Makefile
@@ -5,3 +5,4 @@ KASAN_SANITIZE := n
obj-$(CONFIG_PPC32) += init_32.o
obj-$(CONFIG_PPC_8xx) += 8xx.o
obj-$(CONFIG_PPC_BOOK3S_32) += book3s_32.o
+obj-$(CONFIG_PPC_BOOK3S_64) += init_book3s_64.o
diff --git a/arch/powerpc/mm/kasan/init_book3s_64.c b/arch/powerpc/mm/kasan/init_book3s_64.c
new file mode 100644
index 000000000000..ca913ed951a2
--- /dev/null
+++ b/arch/powerpc/mm/kasan/init_book3s_64.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KASAN for 64-bit Book3S powerpc
+ *
+ * Copyright (C) 2019-2020 IBM Corporation
+ * Author: Daniel Axtens <dja@axtens.net>
+ */
+
+#define DISABLE_BRANCH_PROFILING
+
+#include <linux/kasan.h>
+#include <linux/printk.h>
+#include <linux/sched/task.h>
+#include <linux/memblock.h>
+#include <asm/pgalloc.h>
+
+DEFINE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
+
+static void __init kasan_init_phys_region(void *start, void *end)
+{
+ unsigned long k_start, k_end, k_cur;
+ void *va;
+
+ if (start >= end)
+ return;
+
+ k_start = ALIGN_DOWN((unsigned long)kasan_mem_to_shadow(start), PAGE_SIZE);
+ k_end = ALIGN((unsigned long)kasan_mem_to_shadow(end), PAGE_SIZE);
+
+ va = memblock_alloc(k_end - k_start, PAGE_SIZE);
+ for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE, va += PAGE_SIZE)
+ map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
+}
+
+void __init kasan_init(void)
+{
+ /*
+ * We want to do the following things:
+ * 1) Map real memory into the shadow for all physical memblocks
+ * This takes us from c000... to c008...
+ * 2) Leave a hole over the shadow of vmalloc space. KASAN_VMALLOC
+ * will manage this for us.
+ * This takes us from c008... to c00a...
+ * 3) Map the 'early shadow'/zero page over iomap and vmemmap space.
+ * This takes us up to where we start at c00e...
+ */
+
+ void *k_start = kasan_mem_to_shadow((void *)RADIX_VMALLOC_END);
+ void *k_end = kasan_mem_to_shadow((void *)RADIX_VMEMMAP_END);
+ phys_addr_t start, end;
+ u64 i;
+ pte_t zero_pte = pfn_pte(virt_to_pfn(kasan_early_shadow_page), PAGE_KERNEL);
+
+ if (!early_radix_enabled())
+ panic("KASAN requires radix!");
+
+ for_each_mem_range(i, &start, &end)
+ kasan_init_phys_region((void *)start, (void *)end);
+
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
+ &kasan_early_shadow_pte[i], zero_pte, 0);
+
+ for (i = 0; i < PTRS_PER_PMD; i++)
+ pmd_populate_kernel(&init_mm, &kasan_early_shadow_pmd[i],
+ kasan_early_shadow_pte);
+
+ for (i = 0; i < PTRS_PER_PUD; i++)
+ pud_populate(&init_mm, &kasan_early_shadow_pud[i],
+ kasan_early_shadow_pmd);
+
+ /* map the early shadow over the iomap and vmemmap space */
+ kasan_populate_early_shadow(k_start, k_end);
+
+ /* mark early shadow region as RO and wipe it */
+ zero_pte = pfn_pte(virt_to_pfn(kasan_early_shadow_page), PAGE_KERNEL_RO);
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
+ &kasan_early_shadow_pte[i], zero_pte, 0);
+
+ /*
+ * clear_page relies on some cache info that hasn't been set up yet.
+ * It ends up looping ~forever and blows up other data.
+ * Use memset instead.
+ */
+ memset(kasan_early_shadow_page, 0, PAGE_SIZE);
+
+ static_branch_inc(&powerpc_kasan_enabled_key);
+
+ /* Enable error messages */
+ init_task.kasan_depth = 0;
+ pr_info("KASAN init done (64-bit Book3S)\n");
+}
+
+void __init kasan_late_init(void) { }
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index aca354fb670b..63672aa656e8 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -20,6 +20,7 @@
#include <linux/seq_file.h>
#include <asm/fixmap.h>
#include <linux/const.h>
+#include <linux/kasan.h>
#include <asm/page.h>
#include <asm/hugetlb.h>
@@ -317,6 +318,23 @@ static void walk_pud(struct pg_state *st, p4d_t *p4d, unsigned long start)
unsigned long addr;
unsigned int i;
+#if defined(CONFIG_KASAN) && defined(CONFIG_PPC_BOOK3S_64)
+ /*
+ * On radix + KASAN, we want to check for the KASAN "early" shadow
+ * which covers huge quantities of memory with the same set of
+ * read-only PTEs. If it is, we want to note the first page (to see
+ * the status change), and then note the last page. This gives us good
+ * results without spending ages noting the exact same PTEs over 100s of
+ * terabytes of memory.
+ */
+ if (p4d_page(*p4d) == virt_to_page(lm_alias(kasan_early_shadow_pud))) {
+ walk_pmd(st, pud, start);
+ addr = start + (PTRS_PER_PUD - 1) * PUD_SIZE;
+ walk_pmd(st, pud, addr);
+ return;
+ }
+#endif
+
for (i = 0; i < PTRS_PER_PUD; i++, pud++) {
addr = start + i * PUD_SIZE;
if (!pud_none(*pud) && !pud_is_leaf(*pud))
@@ -387,11 +405,11 @@ static void populate_markers(void)
#endif
address_markers[i++].start_address = FIXADDR_START;
address_markers[i++].start_address = FIXADDR_TOP;
+#endif /* CONFIG_PPC64 */
#ifdef CONFIG_KASAN
address_markers[i++].start_address = KASAN_SHADOW_START;
address_markers[i++].start_address = KASAN_SHADOW_END;
#endif
-#endif /* CONFIG_PPC64 */
}
static int ptdump_show(struct seq_file *m, void *v)
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 3ce907523b1e..9063c13e7221 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -101,6 +101,7 @@ config PPC_BOOK3S_64
select ARCH_SUPPORTS_NUMA_BALANCING
select IRQ_WORK
select PPC_MM_SLICES
+ select KASAN_VMALLOC if KASAN
config PPC_BOOK3E_64
bool "Embedded processors"
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 2eb6ae150d1f..f277e4793696 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,4 +1,10 @@
# SPDX-License-Identifier: GPL-2.0
+
+# nothing that deals with real mode is safe to KASAN
+# in particular, idle code runs a bunch of things in real mode
+KASAN_SANITIZE_idle.o := n
+KASAN_SANITIZE_pci-ioda.o := n
+
obj-y += setup.o opal-call.o opal-wrappers.o opal.o opal-async.o
obj-y += idle.o opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
obj-y += rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index c8a2b0b05ac0..202199ef9e5c 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -30,3 +30,6 @@ obj-$(CONFIG_PPC_SVM) += svm.o
obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
obj-$(CONFIG_SUSPEND) += suspend.o
+
+# nothing that operates in real mode is safe for KASAN
+KASAN_SANITIZE_ras.o := n
--
2.27.0
^ permalink raw reply related
* [PATCH v10 5/6] powerpc/mm/kasan: rename kasan_init_32.c to init_32.c
From: Daniel Axtens @ 2021-02-03 11:59 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: Daniel Axtens
In-Reply-To: <20210203115946.663273-1-dja@axtens.net>
kasan is already implied by the directory name, we don't need to
repeat it.
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
arch/powerpc/mm/kasan/Makefile | 2 +-
arch/powerpc/mm/kasan/{kasan_init_32.c => init_32.c} | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename arch/powerpc/mm/kasan/{kasan_init_32.c => init_32.c} (100%)
diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
index bb1a5408b86b..42fb628a44fd 100644
--- a/arch/powerpc/mm/kasan/Makefile
+++ b/arch/powerpc/mm/kasan/Makefile
@@ -2,6 +2,6 @@
KASAN_SANITIZE := n
-obj-$(CONFIG_PPC32) += kasan_init_32.o
+obj-$(CONFIG_PPC32) += init_32.o
obj-$(CONFIG_PPC_8xx) += 8xx.o
obj-$(CONFIG_PPC_BOOK3S_32) += book3s_32.o
diff --git a/arch/powerpc/mm/kasan/kasan_init_32.c b/arch/powerpc/mm/kasan/init_32.c
similarity index 100%
rename from arch/powerpc/mm/kasan/kasan_init_32.c
rename to arch/powerpc/mm/kasan/init_32.c
--
2.27.0
^ permalink raw reply related
* [PATCH v10 4/6] kasan: Document support on 32-bit powerpc
From: Daniel Axtens @ 2021-02-03 11:59 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: Daniel Axtens
In-Reply-To: <20210203115946.663273-1-dja@axtens.net>
KASAN is supported on 32-bit powerpc and the docs should reflect this.
Document s390 support while we're at it.
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
Documentation/dev-tools/kasan.rst | 7 +++++--
Documentation/powerpc/kasan.txt | 12 ++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
create mode 100644 Documentation/powerpc/kasan.txt
diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index e022b7506e37..9cfc116cc6bf 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -22,7 +22,8 @@ out-of-bounds accesses for global variables is only supported since Clang 11.
Tag-based KASAN is only supported in Clang.
Currently generic KASAN is supported for the x86_64, arm, arm64, xtensa, s390
-and riscv architectures, and tag-based KASAN modes are supported only for arm64.
+and riscv architectures. It is also supported on 32-bit powerpc kernels.
+Tag-based KASAN modes are supported only for arm64.
Usage
-----
@@ -332,7 +333,9 @@ CONFIG_KASAN_VMALLOC
~~~~~~~~~~~~~~~~~~~~
With ``CONFIG_KASAN_VMALLOC``, KASAN can cover vmalloc space at the
-cost of greater memory usage. Currently this is only supported on x86.
+cost of greater memory usage. Currently this supported on x86, s390
+and 32-bit powerpc. It is optional, except on 32-bit powerpc kernels
+with module support, where it is required.
This works by hooking into vmalloc and vmap, and dynamically
allocating real shadow memory to back the mappings.
diff --git a/Documentation/powerpc/kasan.txt b/Documentation/powerpc/kasan.txt
new file mode 100644
index 000000000000..26bb0e8bb18c
--- /dev/null
+++ b/Documentation/powerpc/kasan.txt
@@ -0,0 +1,12 @@
+KASAN is supported on powerpc on 32-bit only.
+
+32 bit support
+==============
+
+KASAN is supported on both hash and nohash MMUs on 32-bit.
+
+The shadow area sits at the top of the kernel virtual memory space above the
+fixmap area and occupies one eighth of the total kernel virtual memory space.
+
+Instrumentation of the vmalloc area is optional, unless built with modules,
+in which case it is required.
--
2.27.0
^ permalink raw reply related
* [PATCH v10 2/6] kasan: allow architectures to provide an outline readiness check
From: Daniel Axtens @ 2021-02-03 11:59 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: Aneesh Kumar K . V, Daniel Axtens
In-Reply-To: <20210203115946.663273-1-dja@axtens.net>
Allow architectures to define a kasan_arch_is_ready() hook that bails
out of any function that's about to touch the shadow unless the arch
says that it is ready for the memory to be accessed. This is fairly
uninvasive and should have a negligible performance penalty.
This will only work in outline mode, so an arch must specify
ARCH_DISABLE_KASAN_INLINE if it requires this.
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Daniel Axtens <dja@axtens.net>
--
I discuss the justfication for this later in the series. Also,
both previous RFCs for ppc64 - by 2 different people - have
needed this trick! See:
- https://lore.kernel.org/patchwork/patch/592820/ # ppc64 hash series
- https://patchwork.ozlabs.org/patch/795211/ # ppc radix series
---
include/linux/kasan.h | 4 ++++
mm/kasan/common.c | 4 ++++
mm/kasan/generic.c | 3 +++
mm/kasan/shadow.c | 4 ++++
4 files changed, 15 insertions(+)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index bb862d1f0e15..d314c0fa5804 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -23,6 +23,10 @@ struct kunit_kasan_expectation {
#endif
+#ifndef kasan_arch_is_ready
+static inline bool kasan_arch_is_ready(void) { return true; }
+#endif
+
#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
#include <linux/pgtable.h>
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index a390fae9d64b..871ceefd723d 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -348,6 +348,10 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,
if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU))
return false;
+ /* We can't read the shadow byte if the arch isn't ready */
+ if (!kasan_arch_is_ready())
+ return false;
+
if (!kasan_byte_accessible(tagged_object)) {
kasan_report_invalid_free(tagged_object, ip);
return true;
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index 2e55e0f82f39..718c171584e3 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -163,6 +163,9 @@ static __always_inline bool check_region_inline(unsigned long addr,
size_t size, bool write,
unsigned long ret_ip)
{
+ if (!kasan_arch_is_ready())
+ return true;
+
if (unlikely(size == 0))
return true;
diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
index de6b3f074742..0aafc2d5138f 100644
--- a/mm/kasan/shadow.c
+++ b/mm/kasan/shadow.c
@@ -85,6 +85,10 @@ void kasan_poison(const void *address, size_t size, u8 value)
address = kasan_reset_tag(address);
size = round_up(size, KASAN_GRANULE_SIZE);
+ /* Don't touch the shadow memory if arch isn't ready */
+ if (!kasan_arch_is_ready())
+ return;
+
/* Skip KFENCE memory if called explicitly outside of sl*b. */
if (is_kfence_address(address))
return;
--
2.27.0
^ permalink raw reply related
* [PATCH v10 3/6] kasan: define and use MAX_PTRS_PER_* for early shadow tables
From: Daniel Axtens @ 2021-02-03 11:59 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: Daniel Axtens
In-Reply-To: <20210203115946.663273-1-dja@axtens.net>
powerpc has a variable number of PTRS_PER_*, set at runtime based
on the MMU that the kernel is booted under.
This means the PTRS_PER_* are no longer constants, and therefore
breaks the build.
Define default MAX_PTRS_PER_*s in the same style as MAX_PTRS_PER_P4D.
As KASAN is the only user at the moment, just define them in the kasan
header, and have them default to PTRS_PER_* unless overridden in arch
code.
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Suggested-by: Balbir Singh <bsingharora@gmail.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
include/linux/kasan.h | 18 +++++++++++++++---
mm/kasan/init.c | 6 +++---
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index d314c0fa5804..84bea59d01b3 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -43,10 +43,22 @@ static inline bool kasan_arch_is_ready(void) { return true; }
#define PTE_HWTABLE_PTRS 0
#endif
+#ifndef MAX_PTRS_PER_PTE
+#define MAX_PTRS_PER_PTE PTRS_PER_PTE
+#endif
+
+#ifndef MAX_PTRS_PER_PMD
+#define MAX_PTRS_PER_PMD PTRS_PER_PMD
+#endif
+
+#ifndef MAX_PTRS_PER_PUD
+#define MAX_PTRS_PER_PUD PTRS_PER_PUD
+#endif
+
extern unsigned char kasan_early_shadow_page[PAGE_SIZE];
-extern pte_t kasan_early_shadow_pte[PTRS_PER_PTE + PTE_HWTABLE_PTRS];
-extern pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD];
-extern pud_t kasan_early_shadow_pud[PTRS_PER_PUD];
+extern pte_t kasan_early_shadow_pte[MAX_PTRS_PER_PTE + PTE_HWTABLE_PTRS];
+extern pmd_t kasan_early_shadow_pmd[MAX_PTRS_PER_PMD];
+extern pud_t kasan_early_shadow_pud[MAX_PTRS_PER_PUD];
extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
int kasan_populate_early_shadow(const void *shadow_start,
diff --git a/mm/kasan/init.c b/mm/kasan/init.c
index c4605ac9837b..b4d822dff1fb 100644
--- a/mm/kasan/init.c
+++ b/mm/kasan/init.c
@@ -41,7 +41,7 @@ static inline bool kasan_p4d_table(pgd_t pgd)
}
#endif
#if CONFIG_PGTABLE_LEVELS > 3
-pud_t kasan_early_shadow_pud[PTRS_PER_PUD] __page_aligned_bss;
+pud_t kasan_early_shadow_pud[MAX_PTRS_PER_PUD] __page_aligned_bss;
static inline bool kasan_pud_table(p4d_t p4d)
{
return p4d_page(p4d) == virt_to_page(lm_alias(kasan_early_shadow_pud));
@@ -53,7 +53,7 @@ static inline bool kasan_pud_table(p4d_t p4d)
}
#endif
#if CONFIG_PGTABLE_LEVELS > 2
-pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD] __page_aligned_bss;
+pmd_t kasan_early_shadow_pmd[MAX_PTRS_PER_PMD] __page_aligned_bss;
static inline bool kasan_pmd_table(pud_t pud)
{
return pud_page(pud) == virt_to_page(lm_alias(kasan_early_shadow_pmd));
@@ -64,7 +64,7 @@ static inline bool kasan_pmd_table(pud_t pud)
return false;
}
#endif
-pte_t kasan_early_shadow_pte[PTRS_PER_PTE + PTE_HWTABLE_PTRS]
+pte_t kasan_early_shadow_pte[MAX_PTRS_PER_PTE + PTE_HWTABLE_PTRS]
__page_aligned_bss;
static inline bool kasan_pte_table(pmd_t pmd)
--
2.27.0
^ permalink raw reply related
* [PATCH v10 1/6] kasan: allow an architecture to disable inline instrumentation
From: Daniel Axtens @ 2021-02-03 11:59 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: Daniel Axtens
In-Reply-To: <20210203115946.663273-1-dja@axtens.net>
For annoying architectural reasons, it's very difficult to support inline
instrumentation on powerpc64.
Add a Kconfig flag to allow an arch to disable inline. (It's a bit
annoying to be 'backwards', but I'm not aware of any way to have
an arch force a symbol to be 'n', rather than 'y'.)
We also disable stack instrumentation in this case as it does things that
are functionally equivalent to inline instrumentation, namely adding
code that touches the shadow directly without going through a C helper.
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
lib/Kconfig.kasan | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index cffc2ebbf185..4f4d3fb8733d 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -12,6 +12,9 @@ config HAVE_ARCH_KASAN_HW_TAGS
config HAVE_ARCH_KASAN_VMALLOC
bool
+config ARCH_DISABLE_KASAN_INLINE
+ def_bool n
+
config CC_HAS_KASAN_GENERIC
def_bool $(cc-option, -fsanitize=kernel-address)
@@ -130,6 +133,7 @@ config KASAN_OUTLINE
config KASAN_INLINE
bool "Inline instrumentation"
+ depends on !ARCH_DISABLE_KASAN_INLINE
help
Compiler directly inserts code checking shadow memory before
memory accesses. This is faster than outline (in some workloads
@@ -141,6 +145,7 @@ endchoice
config KASAN_STACK
bool "Enable stack instrumentation (unsafe)" if CC_IS_CLANG && !COMPILE_TEST
depends on KASAN_GENERIC || KASAN_SW_TAGS
+ depends on !ARCH_DISABLE_KASAN_INLINE
default y if CC_IS_GCC
help
The LLVM stack address sanitizer has a know problem that
@@ -154,6 +159,9 @@ config KASAN_STACK
but clang users can still enable it for builds without
CONFIG_COMPILE_TEST. On gcc it is assumed to always be safe
to use and enabled by default.
+ If the architecture disables inline instrumentation, this is
+ also disabled as it adds inline-style instrumentation that
+ is run unconditionally.
config KASAN_SW_TAGS_IDENTIFY
bool "Enable memory corruption identification"
--
2.27.0
^ permalink raw reply related
* [PATCH v10 0/6] KASAN for powerpc64 radix
From: Daniel Axtens @ 2021-02-03 11:59 UTC (permalink / raw)
To: linux-kernel, linux-mm, linuxppc-dev, kasan-dev, christophe.leroy,
aneesh.kumar, bsingharora
Cc: Daniel Axtens
Building on the work of Christophe, Aneesh and Balbir, I've ported
KASAN to 64-bit Book3S kernels running on the Radix MMU.
v10 rebases on top of next-20210125, fixing things up to work on top
of the latest changes, and fixing some review comments from
Christophe. I have tested host and guest with 64k pages for this spin.
It does not apply to powerpc/next, sorry: there are conflicting kasan
changes staged in next.
There is now only 1 failing KUnit test: kasan_global_oob - gcc puts
the ASAN init code in a section called '.init_array'. Powerpc64 module
loading code goes through and _renames_ any section beginning with
'.init' to begin with '_init' in order to avoid some complexities
around our 24-bit indirect jumps. This means it renames '.init_array'
to '_init_array', and the generic module loading code then fails to
recognise the section as a constructor and thus doesn't run it. This
hack dates back to 2003 and so I'm not going to try to unpick it in
this series. (I suspect this may have previously worked if the code
ended up in .ctors rather than .init_array but I don't keep my old
binaries around so I have no real way of checking.)
(The previously failing stack tests are now skipped due to more
accurate configuration settings.)
Details from v9: This is a significant reworking of the previous
versions. Instead of the previous approach which supported inline
instrumentation, this series provides only outline instrumentation.
To get around the problem of accessing the shadow region inside code we run
with translations off (in 'real mode'), we we restrict checking to when
translations are enabled. This is done via a new hook in the kasan core and
by excluding larger quantites of arch code from instrumentation. The upside
is that we no longer require that you be able to specify the amount of
physically contiguous memory on the system at compile time. Hopefully this
is a better trade-off. More details in patch 6.
kexec works. Both 64k and 4k pages work. Running as a KVM host works, but
nothing in arch/powerpc/kvm is instrumented. It's also potentially a bit
fragile - if any real mode code paths call out to instrumented code, things
will go boom.
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH v9 6/6] powerpc: Book3S 64-bit outline-only KASAN support
From: Daniel Axtens @ 2021-02-03 11:46 UTC (permalink / raw)
To: Christophe Leroy, linux-kernel, linux-mm, linuxppc-dev, kasan-dev,
christophe.leroy, aneesh.kumar, bsingharora
In-Reply-To: <251530bd-49ab-4d6e-13bc-03f97edafcc4@csgroup.eu>
Hi Christophe,
>> select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
>> select HAVE_ARCH_JUMP_LABEL
>> select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
>> - select HAVE_ARCH_KASAN_VMALLOC if PPC32 && PPC_PAGE_SHIFT <= 14
>> + select HAVE_ARCH_KASAN if PPC_BOOK3S_64 && PPC_RADIX_MMU
>
> PPC_RADIX_MMU already depends on PPC_BOOK3S_64 so 'if PPC_RADIX_MMU' would be enough
Done.
>> + select HAVE_ARCH_NO_KASAN_INLINE if PPC_BOOK3S_64 && PPC_RADIX_MMU
>
> This list must respect Alphabetical order.
Fixed.
>
>> + select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN
>> select HAVE_ARCH_KGDB
>> select HAVE_ARCH_MMAP_RND_BITS
>> select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
>> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
>> index b88900f4832f..60c1bba72a6f 100644
>> --- a/arch/powerpc/Kconfig.debug
>> +++ b/arch/powerpc/Kconfig.debug
>> @@ -396,5 +396,5 @@ config PPC_FAST_ENDIAN_SWITCH
>>
>> config KASAN_SHADOW_OFFSET
>> hex
>> - depends on KASAN
>> + depends on KASAN && PPC32
>> default 0xe0000000
>
> Instead of the above, why not doing:
>
> default 0xe0000000 if PPC32
> default 0xa80e000000000000 is PPC_BOOK3S_64
Done. I just used PPC64.
>
>> diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
>> index 73ad038ed10b..105b90594a8a 100644
>> --- a/arch/powerpc/include/asm/book3s/64/hash.h
>> +++ b/arch/powerpc/include/asm/book3s/64/hash.h
>> @@ -18,6 +18,10 @@
>> #include <asm/book3s/64/hash-4k.h>
>> #endif
>>
>> +#define H_PTRS_PER_PTE (1 << H_PTE_INDEX_SIZE)
>> +#define H_PTRS_PER_PMD (1 << H_PMD_INDEX_SIZE)
>> +#define H_PTRS_PER_PUD (1 << H_PUD_INDEX_SIZE)
>> +
>> /* Bits to set in a PMD/PUD/PGD entry valid bit*/
>> #define HASH_PMD_VAL_BITS (0x8000000000000000UL)
>> #define HASH_PUD_VAL_BITS (0x8000000000000000UL)
>> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
>> index a39886681629..767e239d75e3 100644
>> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
>> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
>> @@ -230,6 +230,13 @@ extern unsigned long __pmd_frag_size_shift;
>> #define PTRS_PER_PUD (1 << PUD_INDEX_SIZE)
>> #define PTRS_PER_PGD (1 << PGD_INDEX_SIZE)
>>
>> +#define MAX_PTRS_PER_PTE ((H_PTRS_PER_PTE > R_PTRS_PER_PTE) ? \
>> + H_PTRS_PER_PTE : R_PTRS_PER_PTE)
>
> Nowadays we allow 100 chars per line. Could this fit on a single line ?
Yes, so long as we drop the tab between the name and the definition and
replace it with a single space.
>> +#define MAX_PTRS_PER_PMD ((H_PTRS_PER_PMD > R_PTRS_PER_PMD) ? \
>> + H_PTRS_PER_PMD : R_PTRS_PER_PMD)
>> +#define MAX_PTRS_PER_PUD ((H_PTRS_PER_PUD > R_PTRS_PER_PUD) ? \
>> + H_PTRS_PER_PUD : R_PTRS_PER_PUD)
>> +
>> /* PMD_SHIFT determines what a second-level page table entry can map */
>> #define PMD_SHIFT (PAGE_SHIFT + PTE_INDEX_SIZE)
>> #define PMD_SIZE (1UL << PMD_SHIFT)
>> +#ifdef CONFIG_PPC32
>> #define KASAN_SHADOW_END (-(-KASAN_SHADOW_START >> KASAN_SHADOW_SCALE_SHIFT))
>> +#endif
>> +
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> +/*
>> + * We define the offset such that the shadow of the linear map lives
>> + * at the end of vmemmap space, that is, we choose offset such that
>> + * shadow(c000_0000_0000_0000) = c00e_0000_0000_0000. This gives:
>> + * c00e000000000000 - c000000000000000 >> 3 = a80e000000000000
>> + */
>> +#define KASAN_SHADOW_OFFSET ASM_CONST(0xa80e000000000000)
>
> Why can't you use CONFIG_KASAN_SHADOW_OFFSET ?
I didn't do this earlier due to some interesting interactions in the
generic configuration. The generic Kconfig/Makefile will turn on stack
instrumentation if CONFIG_KASAN_SHADOW_OFFSET is set. This, however,
creates a bunch of inline code that crashes on ppc64 when it runs
translations off.
I've made changes to the patch that introduces ARCH_DISABLE_KASAN_INLINE
to fix this, and converted to using CONFIG_KASAN_SHADOW_OFFSET.
>> +
>> +/*
>> + * The shadow ends before the highest accessible address
>> + * because we don't need a shadow for the shadow. Instead:
>> + * c00e000000000000 << 3 + a80e000000000000000 = c00fc00000000000
>> + */
>> +#define KASAN_SHADOW_END 0xc00fc00000000000UL
>
> I think we should be able to have a common formula for PPC32 and PPC64.
>
Perhaps, but I can't figure out what it would be. For PPC64,
end = start + (size of kernel space + vmalloc space + ioremap space + vmemmap space) >> 3
For PPC32, AIUI you don't have vmemmap space so I'm not sure how to
calculate it.
>> +
>> +DECLARE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
>> +
>> +static inline bool kasan_arch_is_ready_ppc64(void)
>
> I'd make it __always_inline
Done.
>> +{
>> + if (static_branch_likely(&powerpc_kasan_enabled_key))
>> + return true;
>> + return false;
>> +}
>> +
>> +#define kasan_arch_is_ready kasan_arch_is_ready_ppc64
>
> Usually we keep the generic name, you don't need to have an arch specific name.
Done.
>> +#endif
>>
>> #ifdef CONFIG_KASAN
>> void kasan_early_init(void);
>> @@ -47,5 +79,5 @@ void kasan_update_early_region(unsigned long k_start, unsigned long k_end, pte_t
>> int kasan_init_shadow_page_tables(unsigned long k_start, unsigned long k_end);
>> int kasan_init_region(void *start, size_t size);
>>
>> -#endif /* __ASSEMBLY */
>> +#endif /* !__ASSEMBLY__ */
>
> This patch is already big. Is that worth it ?
Fair enough. Dropped.
>> #endif
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index fe2ef598e2ea..cd58202459dd 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -32,6 +32,11 @@ KASAN_SANITIZE_early_32.o := n
>> KASAN_SANITIZE_cputable.o := n
>> KASAN_SANITIZE_prom_init.o := n
>> KASAN_SANITIZE_btext.o := n
>> +KASAN_SANITIZE_paca.o := n
>> +KASAN_SANITIZE_setup_64.o := n
>
> The entire setup_64 ?
> Can you split things out into an early_64.o like was done for ppc32 ?
setup_64.c contains around 40 functions, of which about half are not __init:
cpu_ready_for_interrupts
smp setup/maintenance functions:
early_setup_secondary
panic_smp_self_stop
use_spinloop
smp_release_cpus
(Some of these we couldn't instrument anyway as they use real addresses.)
Functions which AFAICT should be marked __init, as they're static and
only called from __init functions:
init_cache_info
pcpu_cpu_distance
Tiny functions:
memory_block_size_bytes
hw_nmi_get_sample_period
Flush maintenance:
do_nothing (an empty function)
init_fallback_flush
setup_{rfi,entry,uaccess}_flush (3 functions)
{rfi,entry,uaccess}_flush_{get,set,enable} (9 functions)
We don't really get much value from instrumenting any of these, but we
could split out at least the flush maintenance ones if we really wanted
to.
I'd say there isn't enough value in moving them to justify the churn but
I'm happy to move them if people feel differently.
>> +KASAN_SANITIZE_mce.o := n
>> +KASAN_SANITIZE_traps.o := n
>
> Why ? ppc32 doesn't need that.
Some of that code runs with translations off. If we try to access the
shadow region with translations off, things will crash. This is part of
what I gave up in order to be able to have the one kernel binary work
regardless of the quantity of physical memory.
Only traps.c is compiled for ppc32, I'll wrap it so that it's still
instrumented there.
> +KASAN_SANITIZE_mce_power.o := n
>>
>> ifdef CONFIG_KASAN
>> CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
>> +
>> + va = memblock_alloc(k_end - k_start, PAGE_SIZE);
>> + for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
>> + map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
>> + va += PAGE_SIZE;
>> + }
>
> What about:
>
> for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE, va += PAGE_SIZE)
> map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
>
Done.
>> +}
>> +
>> +void __init kasan_init(void)
>> +{
>> + /*
>> + * We want to do the following things:
>> + * 1) Map real memory into the shadow for all physical memblocks
>> + * This takes us from c000... to c008...
>> + * 2) Leave a hole over the shadow of vmalloc space. KASAN_VMALLOC
>> + * will manage this for us.
>> + * This takes us from c008... to c00a...
>> + * 3) Map the 'early shadow'/zero page over iomap and vmemmap space.
>> + * This takes us up to where we start at c00e...
>> + */
>> +
>> + void *k_start = kasan_mem_to_shadow((void *)RADIX_VMALLOC_END);
>> + void *k_end = kasan_mem_to_shadow((void *)RADIX_VMEMMAP_END);
>> + phys_addr_t start, end;
>> + u64 i;
>> + pte_t zero_pte = pfn_pte(virt_to_pfn(kasan_early_shadow_page), PAGE_KERNEL);
>> +
>> + if (!early_radix_enabled())
>> + panic("KASAN requires radix!");
>> +
>> + for_each_mem_range(i, &start, &end) {
>> + kasan_init_phys_region((void *)start, (void *)end);
>> + }
>
> No need of { } for single line loops. Check the kernel codyign stype
Fixed.
>> +
>> + for (i = 0; i < PTRS_PER_PTE; i++)
>> + __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
>> + &kasan_early_shadow_pte[i], zero_pte, 0);
>> +
>> + for (i = 0; i < PTRS_PER_PMD; i++)
>> + pmd_populate_kernel(&init_mm, &kasan_early_shadow_pmd[i],
>> + kasan_early_shadow_pte);
>> +
>> + for (i = 0; i < PTRS_PER_PUD; i++)
>> + pud_populate(&init_mm, &kasan_early_shadow_pud[i],
>> + kasan_early_shadow_pmd);
>> +
>> + /* map the early shadow over the iomap and vmemmap space */
>> + kasan_populate_early_shadow(k_start, k_end);
>> +
>> + /* mark early shadow region as RO and wipe it */
>> + zero_pte = pfn_pte(virt_to_pfn(kasan_early_shadow_page), PAGE_KERNEL_RO);
>> + for (i = 0; i < PTRS_PER_PTE; i++)
>> + __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
>> + &kasan_early_shadow_pte[i], zero_pte, 0);
>> +
>> + /*
>> + * clear_page relies on some cache info that hasn't been set up yet.
>> + * It ends up looping ~forever and blows up other data.
>> + * Use memset instead.
>> + */
>> + memset(kasan_early_shadow_page, 0, PAGE_SIZE);
>> +
>> + static_branch_inc(&powerpc_kasan_enabled_key);
>> +
>> + /* Enable error messages */
>> + init_task.kasan_depth = 0;
>> + pr_info("KASAN init done (64-bit Book3S)\n");
>> +}
>> +
>> +void __init kasan_late_init(void) { }
>> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
>> index aca354fb670b..63672aa656e8 100644
>> --- a/arch/powerpc/mm/ptdump/ptdump.c
>> +++ b/arch/powerpc/mm/ptdump/ptdump.c
>> @@ -20,6 +20,7 @@
>> #include <linux/seq_file.h>
>> #include <asm/fixmap.h>
>> #include <linux/const.h>
>> +#include <linux/kasan.h>
>> #include <asm/page.h>
>> #include <asm/hugetlb.h>
>>
>> @@ -317,6 +318,23 @@ static void walk_pud(struct pg_state *st, p4d_t *p4d, unsigned long start)
>> unsigned long addr;
>> unsigned int i;
>>
>> +#if defined(CONFIG_KASAN) && defined(CONFIG_PPC_BOOK3S_64)
>> + /*
>> + * On radix + KASAN, we want to check for the KASAN "early" shadow
>> + * which covers huge quantities of memory with the same set of
>> + * read-only PTEs. If it is, we want to note the first page (to see
>> + * the status change), and then note the last page. This gives us good
>> + * results without spending ages noting the exact same PTEs over 100s of
>> + * terabytes of memory.
>> + */
>> + if (p4d_page(*p4d) == virt_to_page(lm_alias(kasan_early_shadow_pud))) {
>> + walk_pmd(st, pud, start);
>> + addr = start + (PTRS_PER_PUD - 1) * PUD_SIZE;
>> + walk_pmd(st, pud, addr);
>> + return;
>> + }
>> +#endif
>
> Why do you need that ? When PTEs are all pointing to the same page, it shoud already appear in a
> single line into []
It would eventually be printed as one line, but first you have to
traverse ~7.5 billion PTEs. It's the process of walking that that takes
too long, not the printing.
Thanks for the review, I'll have a new spin out soon.
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH] powerpc/64/signal: Fix regression in __kernel_sigtramp_rt64 semantics
From: Michael Ellerman @ 2021-02-03 11:46 UTC (permalink / raw)
To: Raoni Fassina Firmino, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20210201200505.iz46ubcizipnkcxe@work-tp>
On Mon, 1 Feb 2021 17:05:05 -0300, Raoni Fassina Firmino wrote:
> Tested on powerpc64 and powerpc64le, with a glibc build and running the
> affected glibc's testcase[2], inspected that glibc's backtrace() now gives
> the correct result and gdb backtrace also keeps working as before.
>
> I believe this should be backported to releases 5.9 and 5.10 as userspace
> is affected in this releases.
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/64/signal: Fix regression in __kernel_sigtramp_rt64() semantics
https://git.kernel.org/powerpc/c/24321ac668e452a4942598533d267805f291fdc9
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/sstep: Fix array out of bound warning
From: Michael Ellerman @ 2021-02-03 11:46 UTC (permalink / raw)
To: Ravi Bangoria, mpe; +Cc: naveen.n.rao, jniethe5, paulus, linuxppc-dev
In-Reply-To: <20210129071745.111466-1-ravi.bangoria@linux.ibm.com>
On Fri, 29 Jan 2021 12:47:45 +0530, Ravi Bangoria wrote:
> Compiling kernel with -Warray-bounds throws below warning:
>
> In function 'emulate_vsx_store':
> warning: array subscript is above array bounds [-Warray-bounds]
> buf.d[2] = byterev_8(reg->d[1]);
> ~~~~~^~~
> buf.d[3] = byterev_8(reg->d[0]);
> ~~~~~^~~
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/sstep: Fix array out of bound warning
https://git.kernel.org/powerpc/c/344717a14cd7272f88346022a77742323346299e
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/64s: prevent recursive replay_soft_interrupts causing superfluous interrupt
From: Michael Ellerman @ 2021-02-03 11:46 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Athira Rajeev
In-Reply-To: <20210123061244.2076145-1-npiggin@gmail.com>
On Sat, 23 Jan 2021 16:12:44 +1000, Nicholas Piggin wrote:
> When an asynchronous interrupt calls irq_exit, it checks for softirqs
> that may have been created, and runs them. Running softirqs enables
> local irqs, which can replay pending interrupts causing recursion in
> replay_soft_interrupts. This abridged trace shows how this can occur:
>
> ! NIP replay_soft_interrupts
> LR interrupt_exit_kernel_prepare
> Call Trace:
> interrupt_exit_kernel_prepare (unreliable)
> interrupt_return
> --- interrupt: ea0 at __rb_reserve_next
> NIP __rb_reserve_next
> LR __rb_reserve_next
> Call Trace:
> ring_buffer_lock_reserve
> trace_function
> function_trace_call
> ftrace_call
> __do_softirq
> irq_exit
> timer_interrupt
> ! replay_soft_interrupts
> interrupt_exit_kernel_prepare
> interrupt_return
> --- interrupt: ea0 at arch_local_irq_restore
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/64s: prevent recursive replay_soft_interrupts causing superfluous interrupt
https://git.kernel.org/powerpc/c/4025c784c573cab7e3f84746cc82b8033923ec62
cheers
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o
From: Michael Ellerman @ 2021-02-03 11:46 UTC (permalink / raw)
To: Michael Ellerman, Paul Mackerras, Benjamin Herrenschmidt,
Masahiro Yamada, linuxppc-dev
Cc: Ravi Bangoria, linux-kernel, Nicholas Piggin,
Oliver O'Halloran, Greentime Hu, Michal Suchanek,
Ard Biesheuvel, Daniel Axtens
In-Reply-To: <20201223171142.707053-1-masahiroy@kernel.org>
On Thu, 24 Dec 2020 02:11:41 +0900, Masahiro Yamada wrote:
> vgettimeofday.o is unnecessarily rebuilt. Adding it to 'targets' is not
> enough to fix the issue. Kbuild is correctly rebuilding it because the
> command line is changed.
>
> PowerPC builds each vdso directory twice; first in vdso_prepare to
> generate vdso{32,64}-offsets.h, second as part of the ordinary build
> process to embed vdso{32,64}.so.dbg into the kernel.
>
> [...]
Applied to powerpc/fixes.
[1/2] powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o
https://git.kernel.org/powerpc/c/bce74491c3008e27dd6e8f79a83b4faa77a08f7e
[2/2] powerpc/vdso64: remove meaningless vgettimeofday.o build rule
https://git.kernel.org/powerpc/c/66f0a9e058fad50e569ad752be72e52701991fd5
cheers
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox