From: Ariel Miculas <ariel.miculas@belden.com>
To: linuxppc-dev@lists.ozlabs.org, christian.johannes@belden.com
Cc: Ariel Miculas <ariel.miculas@belden.com>
Subject: [PATCH] powerpc/ptrace: Fix buffer overflow when handling PTRACE_PEEKUSER and PTRACE_POKEUSER
Date: Thu, 9 Jun 2022 12:52:35 +0300 [thread overview]
Message-ID: <20220609095235.37863-1-ariel.miculas@belden.com> (raw)
This fixes the gdbserver issue on PPC32 described here:
Link: https://linuxppc-dev.ozlabs.narkive.com/C46DRek4/debug-problems-on-ppc-83xx-target-due-to-changed-struct-task-struct
On PPC32, the user space code considers the floating point to be an
array of unsigned int (32 bits) - the index passed in is based on
this assumption.
fp_state is a matrix consisting of 32 lines
/* FP and VSX 0-31 register set /
struct thread_fp_state {
u64 fpr[32][TS_FPRWIDTH] attribute((aligned(16)));
u64 fpscr; / Floating point status */
};
On PPC32, PT_FPSCR is defined as: (PT_FPR0 + 2*32 + 1)
This means the fpr index validation allows a range from 0 to 65, leading
to out-of-bounds array access. This ends up corrupting
threads_struct->state, which holds the state of the task. Thus, threads
incorrectly transition from a running state to a traced state and get
stuck in that state.
On PPC32 it's ok to assume that TS_FPRWIDTH is 1 because CONFIG_VSX is
PPC64 specific. TS_FPROFFSET can be safely ignored, thus the assumption
that fpr is an array of 32 elements of type u64 holds true.
Solution taken from arch/powerpc/kernel/ptrace32.c
Signed-off-by: Ariel Miculas <ariel.miculas@belden.com>
---
arch/powerpc/kernel/ptrace/ptrace-fpu.c | 31 +++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-fpu.c b/arch/powerpc/kernel/ptrace/ptrace-fpu.c
index 5dca19361316..93695abbbdfb 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-fpu.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-fpu.c
@@ -6,9 +6,16 @@
#include "ptrace-decl.h"
+#ifdef CONFIG_PPC32
+/* Macros to workout the correct index for the FPR in the thread struct */
+#define FPRNUMBER(i) (((i) - PT_FPR0) >> 1)
+#define FPRHALF(i) (((i) - PT_FPR0) & 1)
+#define FPRINDEX(i) TS_FPRWIDTH * FPRNUMBER(i) * 2 + FPRHALF(i)
+#endif
+
int ptrace_get_fpr(struct task_struct *child, int index, unsigned long *data)
{
-#ifdef CONFIG_PPC_FPU_REGS
+#if defined(CONFIG_PPC_FPU_REGS) && !defined(CONFIG_PPC32)
unsigned int fpidx = index - PT_FPR0;
#endif
@@ -17,10 +24,20 @@ int ptrace_get_fpr(struct task_struct *child, int index, unsigned long *data)
#ifdef CONFIG_PPC_FPU_REGS
flush_fp_to_thread(child);
+#ifdef CONFIG_PPC32
+ /*
+ * the user space code considers the floating point
+ * to be an array of unsigned int (32 bits) - the
+ * index passed in is based on this assumption.
+ */
+ *data = ((unsigned int *)child->thread.fp_state.fpr)
+ [FPRINDEX(index)];
+#else
if (fpidx < (PT_FPSCR - PT_FPR0))
memcpy(data, &child->thread.TS_FPR(fpidx), sizeof(long));
else
*data = child->thread.fp_state.fpscr;
+#endif
#else
*data = 0;
#endif
@@ -30,7 +47,7 @@ int ptrace_get_fpr(struct task_struct *child, int index, unsigned long *data)
int ptrace_put_fpr(struct task_struct *child, int index, unsigned long data)
{
-#ifdef CONFIG_PPC_FPU_REGS
+#if defined(CONFIG_PPC_FPU_REGS) && !defined(CONFIG_PPC32)
unsigned int fpidx = index - PT_FPR0;
#endif
@@ -39,10 +56,20 @@ int ptrace_put_fpr(struct task_struct *child, int index, unsigned long data)
#ifdef CONFIG_PPC_FPU_REGS
flush_fp_to_thread(child);
+#ifdef CONFIG_PPC32
+ /*
+ * the user space code considers the floating point
+ * to be an array of unsigned int (32 bits) - the
+ * index passed in is based on this assumption.
+ */
+ ((unsigned int *)child->thread.fp_state.fpr)
+ [FPRINDEX(index)] = data;
+#else
if (fpidx < (PT_FPSCR - PT_FPR0))
memcpy(&child->thread.TS_FPR(fpidx), &data, sizeof(long));
else
child->thread.fp_state.fpscr = data;
+#endif
#endif
return 0;
--
2.36.1
next reply other threads:[~2022-06-10 11:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-09 9:52 Ariel Miculas [this message]
2022-06-11 9:07 ` [PATCH] powerpc/ptrace: Fix buffer overflow when handling PTRACE_PEEKUSER and PTRACE_POKEUSER Christophe Leroy
[not found] <20220602094746.262360-1-ariel.miculas@belden.com>
[not found] ` <878rqfpb5f.fsf@mpe.ellerman.id.au>
[not found] ` <PH0PR18MB50697FF9DE4A241F3F498F2080DE9@PH0PR18MB5069.namprd18.prod.outlook.com>
[not found] ` <87o7z5ono9.fsf@mpe.ellerman.id.au>
2022-06-09 9:20 ` Christophe Leroy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220609095235.37863-1-ariel.miculas@belden.com \
--to=ariel.miculas@belden.com \
--cc=christian.johannes@belden.com \
--cc=linuxppc-dev@lists.ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).