linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes
@ 2017-12-11 22:50 Maciej W. Rozycki
  2017-12-11 22:51 ` [PATCH v2 1/6] MIPS: Factor out NT_PRFPREG regset access helpers Maciej W. Rozycki
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2017-12-11 22:50 UTC (permalink / raw)
  To: Ralf Baechle, James Hogan
  Cc: Paul Burton, Alex Smith, Dave Martin, linux-mips, linux-kernel, stable

Hi,

 This series corrects a number of issues with NT_PRFPREG regset, most 
importantly an FCSR access API regression introduced with the addition of 
MSA support, and then a few smaller issues with the get/set handlers.

 I have decided to factor out non-MSA and MSA context helpers as the first 
step to avoid the issue with excessive indentation that would inevitably 
happen if the regression fix was applied to current code as it stands.  
It shouldn't be a big deal with backporting as this code hasn't changed 
much since the regression, and it will make any future bacports easier.  
Only a call to `init_fp_ctx' will have to be trivially resolved (though 
arguably commit ac9ad83bc318 ("MIPS: prevent FP context set via ptrace 
being discarded"), which has added `init_fp_ctx', would be good to 
backport as far as possible instead).

 These changes have been verified by examining the register state recorded 
in core dumps manually with GDB, as well as by running the GDB test suite.  
No user of ptrace(2) PTRACE_GETREGSET and PTRACE_SETREGSET requests is 
known for the MIPS port, so this part remains not covered, however it is 
assumed to remain consistent with how the creation of core file works.

 See individual patch descriptions for further details, and for changes 
made since v1 to address concerns raised in the review.

  Maciej

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/6] MIPS: Factor out NT_PRFPREG regset access helpers
  2017-12-11 22:50 [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Maciej W. Rozycki
@ 2017-12-11 22:51 ` Maciej W. Rozycki
  2017-12-11 22:52 ` [PATCH v2 2/6] MIPS: Guard against any partial write attempt with PTRACE_SETREGSET Maciej W. Rozycki
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2017-12-11 22:51 UTC (permalink / raw)
  To: Ralf Baechle, James Hogan
  Cc: Paul Burton, Alex Smith, Dave Martin, linux-mips, linux-kernel, stable

In preparation to fix a commit 72b22bbad1e7 ("MIPS: Don't assume 64-bit 
FP registers for FP regset") FCSR access regression factor out 
NT_PRFPREG regset access helpers for the non-MSA and the MSA variants 
respectively, to avoid having to deal with excessive indentation in the
actual fix.

No functional change, however use `target->thread.fpu.fpr[0]' rather 
than `target->thread.fpu.fpr[i]' for FGR holding type size determination 
as there's no `i' variable to refer to anymore, and for the factored out 
`i' variable declaration use `unsigned int' rather than `unsigned' as 
its type, following the common style.

Cc: stable@vger.kernel.org # v3.15+
Fixes: 72b22bbad1e7 ("MIPS: Don't assume 64-bit FP registers for FP regset")
Signed-off-by: Maciej W. Rozycki <macro@mips.com>
---

No changes from v1.

---
 arch/mips/kernel/ptrace.c |  108 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 83 insertions(+), 25 deletions(-)

linux-mips-nt-prfpreg-factor-out.diff
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2017-11-23 19:18:08.831530000 +0000
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2017-11-28 22:44:11.537151000 +0000
@@ -410,25 +410,36 @@ static int gpr64_set(struct task_struct 
 
 #endif /* CONFIG_64BIT */
 
-static int fpr_get(struct task_struct *target,
-		   const struct user_regset *regset,
-		   unsigned int pos, unsigned int count,
-		   void *kbuf, void __user *ubuf)
+/*
+ * Copy the floating-point context to the supplied NT_PRFPREG buffer,
+ * !CONFIG_CPU_HAS_MSA variant.  FP context's general register slots
+ * correspond 1:1 to buffer slots.
+ */
+static int fpr_get_fpa(struct task_struct *target,
+		       unsigned int *pos, unsigned int *count,
+		       void **kbuf, void __user **ubuf)
 {
-	unsigned i;
-	int err;
-	u64 fpr_val;
-
-	/* XXX fcr31  */
+	return user_regset_copyout(pos, count, kbuf, ubuf,
+				   &target->thread.fpu,
+				   0, sizeof(elf_fpregset_t));
+}
 
-	if (sizeof(target->thread.fpu.fpr[i]) == sizeof(elf_fpreg_t))
-		return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
-					   &target->thread.fpu,
-					   0, sizeof(elf_fpregset_t));
+/*
+ * Copy the floating-point context to the supplied NT_PRFPREG buffer,
+ * CONFIG_CPU_HAS_MSA variant.  Only lower 64 bits of FP context's
+ * general register slots are copied to buffer slots.
+ */
+static int fpr_get_msa(struct task_struct *target,
+		       unsigned int *pos, unsigned int *count,
+		       void **kbuf, void __user **ubuf)
+{
+	unsigned int i;
+	u64 fpr_val;
+	int err;
 
 	for (i = 0; i < NUM_FPU_REGS; i++) {
 		fpr_val = get_fpr64(&target->thread.fpu.fpr[i], 0);
-		err = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+		err = user_regset_copyout(pos, count, kbuf, ubuf,
 					  &fpr_val, i * sizeof(elf_fpreg_t),
 					  (i + 1) * sizeof(elf_fpreg_t));
 		if (err)
@@ -438,27 +449,54 @@ static int fpr_get(struct task_struct *t
 	return 0;
 }
 
-static int fpr_set(struct task_struct *target,
+/* Copy the floating-point context to the supplied NT_PRFPREG buffer.  */
+static int fpr_get(struct task_struct *target,
 		   const struct user_regset *regset,
 		   unsigned int pos, unsigned int count,
-		   const void *kbuf, const void __user *ubuf)
+		   void *kbuf, void __user *ubuf)
 {
-	unsigned i;
 	int err;
-	u64 fpr_val;
 
 	/* XXX fcr31  */
 
-	init_fp_ctx(target);
+	if (sizeof(target->thread.fpu.fpr[0]) == sizeof(elf_fpreg_t))
+		err = fpr_get_fpa(target, &pos, &count, &kbuf, &ubuf);
+	else
+		err = fpr_get_msa(target, &pos, &count, &kbuf, &ubuf);
 
-	if (sizeof(target->thread.fpu.fpr[i]) == sizeof(elf_fpreg_t))
-		return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
-					  &target->thread.fpu,
-					  0, sizeof(elf_fpregset_t));
+	return err;
+}
+
+/*
+ * Copy the supplied NT_PRFPREG buffer to the floating-point context,
+ * !CONFIG_CPU_HAS_MSA variant.   Buffer slots correspond 1:1 to FP
+ * context's general register slots.
+ */
+static int fpr_set_fpa(struct task_struct *target,
+		       unsigned int *pos, unsigned int *count,
+		       const void **kbuf, const void __user **ubuf)
+{
+	return user_regset_copyin(pos, count, kbuf, ubuf,
+				  &target->thread.fpu,
+				  0, sizeof(elf_fpregset_t));
+}
+
+/*
+ * Copy the supplied NT_PRFPREG buffer to the floating-point context,
+ * CONFIG_CPU_HAS_MSA variant.  Buffer slots are copied to lower 64
+ * bits only of FP context's general register slots.
+ */
+static int fpr_set_msa(struct task_struct *target,
+		       unsigned int *pos, unsigned int *count,
+		       const void **kbuf, const void __user **ubuf)
+{
+	unsigned int i;
+	u64 fpr_val;
+	int err;
 
 	BUILD_BUG_ON(sizeof(fpr_val) != sizeof(elf_fpreg_t));
-	for (i = 0; i < NUM_FPU_REGS && count >= sizeof(elf_fpreg_t); i++) {
-		err = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+	for (i = 0; i < NUM_FPU_REGS && *count >= sizeof(elf_fpreg_t); i++) {
+		err = user_regset_copyin(pos, count, kbuf, ubuf,
 					 &fpr_val, i * sizeof(elf_fpreg_t),
 					 (i + 1) * sizeof(elf_fpreg_t));
 		if (err)
@@ -469,6 +507,26 @@ static int fpr_set(struct task_struct *t
 	return 0;
 }
 
+/* Copy the supplied NT_PRFPREG buffer to the floating-point context.  */
+static int fpr_set(struct task_struct *target,
+		   const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   const void *kbuf, const void __user *ubuf)
+{
+	int err;
+
+	/* XXX fcr31  */
+
+	init_fp_ctx(target);
+
+	if (sizeof(target->thread.fpu.fpr[0]) == sizeof(elf_fpreg_t))
+		err = fpr_set_fpa(target, &pos, &count, &kbuf, &ubuf);
+	else
+		err = fpr_set_msa(target, &pos, &count, &kbuf, &ubuf);
+
+	return err;
+}
+
 enum mips_regset {
 	REGSET_GPR,
 	REGSET_FPR,

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/6] MIPS: Guard against any partial write attempt with PTRACE_SETREGSET
  2017-12-11 22:50 [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Maciej W. Rozycki
  2017-12-11 22:51 ` [PATCH v2 1/6] MIPS: Factor out NT_PRFPREG regset access helpers Maciej W. Rozycki
@ 2017-12-11 22:52 ` Maciej W. Rozycki
  2017-12-11 22:53 ` [PATCH v2 3/6] MIPS: Consistently handle buffer counter " Maciej W. Rozycki
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2017-12-11 22:52 UTC (permalink / raw)
  To: Ralf Baechle, James Hogan
  Cc: Paul Burton, Alex Smith, Dave Martin, linux-mips, linux-kernel, stable

Complement commit d614fd58a283 ("mips/ptrace: Preserve previous 
registers for short regset write") and ensure that no partial register 
write attempt is made with PTRACE_SETREGSET, as we do not preinitialize 
any temporaries used to hold incoming register data and consequently 
random data could be written.

It is the responsibility of the caller, such as `ptrace_regset', to 
arrange for writes to span whole registers only, so here we only assert 
that it has indeed happened.

Cc: stable@vger.kernel.org # v3.15+
Fixes: 72b22bbad1e7 ("MIPS: Don't assume 64-bit FP registers for FP regset")
Signed-off-by: Maciej W. Rozycki <macro@mips.com>
---

New in v2.

---
 arch/mips/kernel/ptrace.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

linux-mips-nt-prfpreg-size-bug.diff
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2017-12-08 16:21:09.783307000 +0000
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2017-12-08 16:21:19.498382000 +0000
@@ -507,7 +507,15 @@ static int fpr_set_msa(struct task_struc
 	return 0;
 }
 
-/* Copy the supplied NT_PRFPREG buffer to the floating-point context.  */
+/*
+ * Copy the supplied NT_PRFPREG buffer to the floating-point context.
+ *
+ * We optimize for the case where `count % sizeof(elf_fpreg_t) == 0',
+ * which is supposed to have been guaranteed by the kernel before
+ * calling us, e.g. in `ptrace_regset'.  We enforce that requirement,
+ * so that we can safely avoid preinitializing temporaries for
+ * partial register writes.
+ */
 static int fpr_set(struct task_struct *target,
 		   const struct user_regset *regset,
 		   unsigned int pos, unsigned int count,
@@ -515,6 +523,8 @@ static int fpr_set(struct task_struct *t
 {
 	int err;
 
+	BUG_ON(count % sizeof(elf_fpreg_t));
+
 	/* XXX fcr31  */
 
 	init_fp_ctx(target);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/6] MIPS: Consistently handle buffer counter with PTRACE_SETREGSET
  2017-12-11 22:50 [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Maciej W. Rozycki
  2017-12-11 22:51 ` [PATCH v2 1/6] MIPS: Factor out NT_PRFPREG regset access helpers Maciej W. Rozycki
  2017-12-11 22:52 ` [PATCH v2 2/6] MIPS: Guard against any partial write attempt with PTRACE_SETREGSET Maciej W. Rozycki
@ 2017-12-11 22:53 ` Maciej W. Rozycki
  2017-12-11 22:54 ` [PATCH v2 4/6] MIPS: Fix an FCSR access API regression with NT_PRFPREG and MSA Maciej W. Rozycki
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2017-12-11 22:53 UTC (permalink / raw)
  To: Ralf Baechle, James Hogan
  Cc: Paul Burton, Alex Smith, Dave Martin, linux-mips, linux-kernel, stable

Update commit d614fd58a283 ("mips/ptrace: Preserve previous registers 
for short regset write") bug and consistently consume all data supplied 
to `fpr_set_msa' with the ptrace(2) PTRACE_SETREGSET request, such that 
a zero data buffer counter is returned where insufficient data has been 
given to fill a whole number of FP general registers.

In reality this is not going to happen, as the caller is supposed to 
only supply data covering a whole number of registers and it is verified 
in `ptrace_regset' and again asserted in `fpr_set', however structuring 
code such that the presence of trailing partial FP general register data 
causes `fpr_set_msa' to return with a non-zero data buffer counter makes 
it appear that this trailing data will be used if there are subsequent 
writes made to FP registers, which is going to be the case with the FCSR 
once the missing write to that register has been fixed.

Cc: stable@vger.kernel.org # v4.11+
Fixes: d614fd58a283 ("mips/ptrace: Preserve previous registers for short regset write")
Signed-off-by: Maciej W. Rozycki <macro@mips.com>
---

Changes from v1:

- reordered in the series,

- heading and description updated to better reflect reality.

---
 arch/mips/kernel/ptrace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

linux-mips-nt-prfpreg-count.diff
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2017-12-08 15:43:29.956644000 +0000
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2017-12-08 15:44:14.013974000 +0000
@@ -495,7 +495,7 @@ static int fpr_set_msa(struct task_struc
 	int err;
 
 	BUILD_BUG_ON(sizeof(fpr_val) != sizeof(elf_fpreg_t));
-	for (i = 0; i < NUM_FPU_REGS && *count >= sizeof(elf_fpreg_t); i++) {
+	for (i = 0; i < NUM_FPU_REGS && *count > 0; i++) {
 		err = user_regset_copyin(pos, count, kbuf, ubuf,
 					 &fpr_val, i * sizeof(elf_fpreg_t),
 					 (i + 1) * sizeof(elf_fpreg_t));

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 4/6] MIPS: Fix an FCSR access API regression with NT_PRFPREG and MSA
  2017-12-11 22:50 [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Maciej W. Rozycki
                   ` (2 preceding siblings ...)
  2017-12-11 22:53 ` [PATCH v2 3/6] MIPS: Consistently handle buffer counter " Maciej W. Rozycki
@ 2017-12-11 22:54 ` Maciej W. Rozycki
  2017-12-11 22:55 ` [PATCH v2 5/6] MIPS: Also verify sizeof `elf_fpreg_t' with PTRACE_SETREGSET Maciej W. Rozycki
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2017-12-11 22:54 UTC (permalink / raw)
  To: Ralf Baechle, James Hogan
  Cc: Paul Burton, Alex Smith, Dave Martin, linux-mips, linux-kernel, stable

Fix a commit 72b22bbad1e7 ("MIPS: Don't assume 64-bit FP registers for 
FP regset") public API regression, then activated by commit 1db1af84d6df 
("MIPS: Basic MSA context switching support"), that caused the FCSR 
register not to be read or written for CONFIG_CPU_HAS_MSA kernel 
configurations (regardless of actual presence or absence of the MSA 
feature in a given processor) with ptrace(2) PTRACE_GETREGSET and 
PTRACE_SETREGSET requests nor recorded in core dumps.

This is because with !CONFIG_CPU_HAS_MSA configurations the whole of 
`elf_fpregset_t' array is bulk-copied as it is, which includes the FCSR 
in one half of the last, 33rd slot, whereas with CONFIG_CPU_HAS_MSA 
configurations array elements are copied individually, and then only the 
leading 32 FGR slots while the remaining slot is ignored.

Correct the code then such that only FGR slots are copied in the 
respective !MSA and MSA helpers an then the FCSR slot is handled 
separately in common code.  Use `ptrace_setfcr31' to update the FCSR 
too, so that the read-only mask is respected.

Retrieving a correct value of FCSR is important in debugging not only 
for the human to be able to get the right interpretation of the 
situation, but for correct operation of GDB as well.  This is because 
the condition code bits in FSCR are used by GDB to determine the 
location to place a breakpoint at when single-stepping through an FPU 
branch instruction.  If such a breakpoint is placed incorrectly (i.e. 
with the condition reversed), then it will be missed, likely causing the 
debuggee to run away from the control of GDB and consequently breaking 
the process of investigation.

Fortunately GDB continues using the older PTRACE_GETFPREGS ptrace(2) 
request which is unaffected, so the regression only really hits with 
post-mortem debug sessions using a core dump file, in which case 
execution, and consequently single-stepping through branches is not 
possible.  Of course core files created by buggy kernels out there will 
have the value of FCSR recorded clobbered, but such core files cannot be 
corrected and the person using them simply will have to be aware that 
the value of FCSR retrieved is not reliable.

Which also means we can likely get away without defining a replacement 
API which would ensure a correct value of FSCR to be retrieved, or none 
at all.

This is based on previous work by Alex Smith, extensively rewritten.

Cc: stable@vger.kernel.org # v3.15+
Fixes: 72b22bbad1e7 ("MIPS: Don't assume 64-bit FP registers for FP regset")
Signed-off-by: Alex Smith <alex@alex-smith.me.uk>
Signed-off-by: James Hogan <james.hogan@mips.com>
Signed-off-by: Maciej W. Rozycki <macro@mips.com>
---

 I think the fix to use `ptrace_setfcr31' could be a separate change, but 
given how its absence has been entangled with the inconsistency between
!MSA and MSA I cannot imagine how to do it in a sane manner:

1. If done first, then it would have to be applied to the !MSA case only,
   only to be moved to shared code in the next step.

2. If done second, then new incorrect shared code would have to be written
   only to be fixed in the next step.

So I think it has to go along with the main fix.  Though maybe choosing #2 
would make backporting easier, as `ptrace_setfcr31' is v4.1+ only and:

		target->thread.fpu.fcr31 = fcr31;

has to be used before that (previously the read-only mask wasn't defined
and you could write arbitrary rubbish to FCSR, especially in the emulated
case).

---

Changes from v1:

- regenerated.

---
 arch/mips/kernel/ptrace.c |   47 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

linux-mips-nt-prfpreg-fcsr.diff
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2017-12-08 16:21:28.000000000 +0000
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2017-12-08 16:22:04.167706000 +0000
@@ -413,7 +413,7 @@ static int gpr64_set(struct task_struct 
 /*
  * Copy the floating-point context to the supplied NT_PRFPREG buffer,
  * !CONFIG_CPU_HAS_MSA variant.  FP context's general register slots
- * correspond 1:1 to buffer slots.
+ * correspond 1:1 to buffer slots.  Only general registers are copied.
  */
 static int fpr_get_fpa(struct task_struct *target,
 		       unsigned int *pos, unsigned int *count,
@@ -421,13 +421,14 @@ static int fpr_get_fpa(struct task_struc
 {
 	return user_regset_copyout(pos, count, kbuf, ubuf,
 				   &target->thread.fpu,
-				   0, sizeof(elf_fpregset_t));
+				   0, NUM_FPU_REGS * sizeof(elf_fpreg_t));
 }
 
 /*
  * Copy the floating-point context to the supplied NT_PRFPREG buffer,
  * CONFIG_CPU_HAS_MSA variant.  Only lower 64 bits of FP context's
- * general register slots are copied to buffer slots.
+ * general register slots are copied to buffer slots.  Only general
+ * registers are copied.
  */
 static int fpr_get_msa(struct task_struct *target,
 		       unsigned int *pos, unsigned int *count,
@@ -449,20 +450,29 @@ static int fpr_get_msa(struct task_struc
 	return 0;
 }
 
-/* Copy the floating-point context to the supplied NT_PRFPREG buffer.  */
+/*
+ * Copy the floating-point context to the supplied NT_PRFPREG buffer.
+ * Choose the appropriate helper for general registers, and then copy
+ * the FCSR register separately.
+ */
 static int fpr_get(struct task_struct *target,
 		   const struct user_regset *regset,
 		   unsigned int pos, unsigned int count,
 		   void *kbuf, void __user *ubuf)
 {
+	const int fcr31_pos = NUM_FPU_REGS * sizeof(elf_fpreg_t);
 	int err;
 
-	/* XXX fcr31  */
-
 	if (sizeof(target->thread.fpu.fpr[0]) == sizeof(elf_fpreg_t))
 		err = fpr_get_fpa(target, &pos, &count, &kbuf, &ubuf);
 	else
 		err = fpr_get_msa(target, &pos, &count, &kbuf, &ubuf);
+	if (err)
+		return err;
+
+	err = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+				  &target->thread.fpu.fcr31,
+				  fcr31_pos, fcr31_pos + sizeof(u32));
 
 	return err;
 }
@@ -470,7 +480,7 @@ static int fpr_get(struct task_struct *t
 /*
  * Copy the supplied NT_PRFPREG buffer to the floating-point context,
  * !CONFIG_CPU_HAS_MSA variant.   Buffer slots correspond 1:1 to FP
- * context's general register slots.
+ * context's general register slots.  Only general registers are copied.
  */
 static int fpr_set_fpa(struct task_struct *target,
 		       unsigned int *pos, unsigned int *count,
@@ -478,13 +488,14 @@ static int fpr_set_fpa(struct task_struc
 {
 	return user_regset_copyin(pos, count, kbuf, ubuf,
 				  &target->thread.fpu,
-				  0, sizeof(elf_fpregset_t));
+				  0, NUM_FPU_REGS * sizeof(elf_fpreg_t));
 }
 
 /*
  * Copy the supplied NT_PRFPREG buffer to the floating-point context,
  * CONFIG_CPU_HAS_MSA variant.  Buffer slots are copied to lower 64
- * bits only of FP context's general register slots.
+ * bits only of FP context's general register slots.  Only general
+ * registers are copied.
  */
 static int fpr_set_msa(struct task_struct *target,
 		       unsigned int *pos, unsigned int *count,
@@ -509,6 +520,8 @@ static int fpr_set_msa(struct task_struc
 
 /*
  * Copy the supplied NT_PRFPREG buffer to the floating-point context.
+ * Choose the appropriate helper for general registers, and then copy
+ * the FCSR register separately.
  *
  * We optimize for the case where `count % sizeof(elf_fpreg_t) == 0',
  * which is supposed to have been guaranteed by the kernel before
@@ -521,18 +534,30 @@ static int fpr_set(struct task_struct *t
 		   unsigned int pos, unsigned int count,
 		   const void *kbuf, const void __user *ubuf)
 {
+	const int fcr31_pos = NUM_FPU_REGS * sizeof(elf_fpreg_t);
+	u32 fcr31;
 	int err;
 
 	BUG_ON(count % sizeof(elf_fpreg_t));
 
-	/* XXX fcr31  */
-
 	init_fp_ctx(target);
 
 	if (sizeof(target->thread.fpu.fpr[0]) == sizeof(elf_fpreg_t))
 		err = fpr_set_fpa(target, &pos, &count, &kbuf, &ubuf);
 	else
 		err = fpr_set_msa(target, &pos, &count, &kbuf, &ubuf);
+	if (err)
+		return err;
+
+	if (count > 0) {
+		err = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+					 &fcr31,
+					 fcr31_pos, fcr31_pos + sizeof(u32));
+		if (err)
+			return err;
+
+		ptrace_setfcr31(target, fcr31);
+	}
 
 	return err;
 }

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 5/6] MIPS: Also verify sizeof `elf_fpreg_t' with PTRACE_SETREGSET
  2017-12-11 22:50 [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Maciej W. Rozycki
                   ` (3 preceding siblings ...)
  2017-12-11 22:54 ` [PATCH v2 4/6] MIPS: Fix an FCSR access API regression with NT_PRFPREG and MSA Maciej W. Rozycki
@ 2017-12-11 22:55 ` Maciej W. Rozycki
  2017-12-11 22:56 ` [PATCH v2 6/6] MIPS: Disallow outsized PTRACE_SETREGSET NT_PRFPREG regset accesses Maciej W. Rozycki
  2017-12-12 18:55 ` [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Ralf Baechle
  6 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2017-12-11 22:55 UTC (permalink / raw)
  To: Ralf Baechle, James Hogan
  Cc: Paul Burton, Alex Smith, Dave Martin, linux-mips, linux-kernel, stable

Complement commit d614fd58a283 ("mips/ptrace: Preserve previous 
registers for short regset write") and like with the PTRACE_GETREGSET 
ptrace(2) request also apply a BUILD_BUG_ON check for the size of the 
`elf_fpreg_t' type in the PTRACE_SETREGSET request handler.

Cc: stable@vger.kernel.org # v4.11+
Fixes: d614fd58a283 ("mips/ptrace: Preserve previous registers for short regset write")
Signed-off-by: Maciej W. Rozycki <macro@mips.com>
---

No changes from v1.

---
 arch/mips/kernel/ptrace.c |    1 +
 1 file changed, 1 insertion(+)

linux-mips-nt-prfpreg-build-bug.diff
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2017-11-28 23:33:33.395023000 +0000
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2017-11-28 23:52:34.944549000 +0000
@@ -438,6 +438,7 @@ static int fpr_get_msa(struct task_struc
 	u64 fpr_val;
 	int err;
 
+	BUILD_BUG_ON(sizeof(fpr_val) != sizeof(elf_fpreg_t));
 	for (i = 0; i < NUM_FPU_REGS; i++) {
 		fpr_val = get_fpr64(&target->thread.fpu.fpr[i], 0);
 		err = user_regset_copyout(pos, count, kbuf, ubuf,

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 6/6] MIPS: Disallow outsized PTRACE_SETREGSET NT_PRFPREG regset accesses
  2017-12-11 22:50 [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Maciej W. Rozycki
                   ` (4 preceding siblings ...)
  2017-12-11 22:55 ` [PATCH v2 5/6] MIPS: Also verify sizeof `elf_fpreg_t' with PTRACE_SETREGSET Maciej W. Rozycki
@ 2017-12-11 22:56 ` Maciej W. Rozycki
  2017-12-12 18:55 ` [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Ralf Baechle
  6 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2017-12-11 22:56 UTC (permalink / raw)
  To: Ralf Baechle, James Hogan
  Cc: Paul Burton, Alex Smith, Dave Martin, linux-mips, linux-kernel, stable

Complement commit c23b3d1a5311 ("MIPS: ptrace: Change GP regset to use 
correct core dump register layout") and also reject outsized 
PTRACE_SETREGSET requests to the NT_PRFPREG regset, like with the 
NT_PRSTATUS regset.

Cc: stable@vger.kernel.org # v3.17+
Fixes: c23b3d1a5311 ("MIPS: ptrace: Change GP regset to use correct core dump register layout")
Signed-off-by: Maciej W. Rozycki <macro@mips.com>
---

Changes from v1:

- regenerated.

---
 arch/mips/kernel/ptrace.c |    3 +++
 1 file changed, 3 insertions(+)

linux-mips-nt-prfpreg-size.diff
Index: linux-sfr-test/arch/mips/kernel/ptrace.c
===================================================================
--- linux-sfr-test.orig/arch/mips/kernel/ptrace.c	2017-12-08 16:22:08.062741000 +0000
+++ linux-sfr-test/arch/mips/kernel/ptrace.c	2017-12-08 16:22:17.727811000 +0000
@@ -541,6 +541,9 @@ static int fpr_set(struct task_struct *t
 
 	BUG_ON(count % sizeof(elf_fpreg_t));
 
+	if (pos + count > sizeof(elf_fpregset_t))
+		return -EIO;
+
 	init_fp_ctx(target);
 
 	if (sizeof(target->thread.fpu.fpr[0]) == sizeof(elf_fpreg_t))

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes
  2017-12-11 22:50 [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Maciej W. Rozycki
                   ` (5 preceding siblings ...)
  2017-12-11 22:56 ` [PATCH v2 6/6] MIPS: Disallow outsized PTRACE_SETREGSET NT_PRFPREG regset accesses Maciej W. Rozycki
@ 2017-12-12 18:55 ` Ralf Baechle
  6 siblings, 0 replies; 8+ messages in thread
From: Ralf Baechle @ 2017-12-12 18:55 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: James Hogan, Paul Burton, Alex Smith, Dave Martin, linux-mips,
	linux-kernel, stable

Thanks, applied.

  Ralf

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-12-12 18:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-11 22:50 [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Maciej W. Rozycki
2017-12-11 22:51 ` [PATCH v2 1/6] MIPS: Factor out NT_PRFPREG regset access helpers Maciej W. Rozycki
2017-12-11 22:52 ` [PATCH v2 2/6] MIPS: Guard against any partial write attempt with PTRACE_SETREGSET Maciej W. Rozycki
2017-12-11 22:53 ` [PATCH v2 3/6] MIPS: Consistently handle buffer counter " Maciej W. Rozycki
2017-12-11 22:54 ` [PATCH v2 4/6] MIPS: Fix an FCSR access API regression with NT_PRFPREG and MSA Maciej W. Rozycki
2017-12-11 22:55 ` [PATCH v2 5/6] MIPS: Also verify sizeof `elf_fpreg_t' with PTRACE_SETREGSET Maciej W. Rozycki
2017-12-11 22:56 ` [PATCH v2 6/6] MIPS: Disallow outsized PTRACE_SETREGSET NT_PRFPREG regset accesses Maciej W. Rozycki
2017-12-12 18:55 ` [PATCH v2 0/6] MIPS: NT_PRFPREG regset handling fixes Ralf Baechle

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).