[PATCH v4] parisc: Fix some apparent put_user() failures

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



After commit 4b9d2a731c3d ("parisc: Switch user access functions
to signal errors in r29 instead of r8") bash suddenly started
to report those warnings after login:

-bash: cannot set terminal process group (-1): Bad file descriptor
-bash: no job control in this shell

It turned out, that a function call inside a put_user(), e.g.:
put_user(vt_do_kdgkbmode(console), (int __user *)arg);
clobbered the error register (r29) and thus the put_user() call itself
seem to have failed. Rearranging the C instructions didn't helped,
because then sometimes the compiler choosed another register which would
break the fault handler.

This patch now takes another and much smarter approach.

Instead of hardcoding a specific register as fault register, leave the
decision up to the compiler. In the load/store assembly we initialize
this register with zero by using "copy %%r0,reg".

In case a fault happens, the fault handler will now read this copy
instruction, extract the used fault register and store the -EFAULT
failure code in it.

Reported-by: John David Anglin <dave.anglin@xxxxxxxx>
Signed-off-by: Helge Deller <deller@xxxxxx>
Fixes: 4b9d2a731c3d ("parisc: Switch user access functions to signal errors in r29 instead of r8")
Signed-off-by: Helge Deller <deller@xxxxxx>
---
 arch/parisc/include/asm/uaccess.h | 35 ++++++++++++++++---------------
 arch/parisc/mm/fault.c            | 31 ++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index ebf8a845b017..bc80d71656bb 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h
@@ -53,18 +53,15 @@ struct exception_table_entry {
 /*
  * ASM_EXCEPTIONTABLE_ENTRY_EFAULT() creates a special exception table entry
  * (with lowest bit set) for which the fault handler in fixup_exception() will
- * load -EFAULT into %r29 for a read or write fault, and zeroes the target
- * register in case of a read fault in get_user().
+ * load -EFAULT into the error register for a read or write fault, and zeroes
+ * the target register in case of a read fault in get_user().
  */
-#define ASM_EXCEPTIONTABLE_REG	29
-#define ASM_EXCEPTIONTABLE_VAR(__variable)		\
-	register long __variable __asm__ ("r29") = 0
 #define ASM_EXCEPTIONTABLE_ENTRY_EFAULT( fault_addr, except_addr )\
 	ASM_EXCEPTIONTABLE_ENTRY( fault_addr, except_addr + 1)

 #define __get_user_internal(sr, val, ptr)		\
 ({							\
-	ASM_EXCEPTIONTABLE_VAR(__gu_err);		\
+	register long __gu_err;				\
 							\
 	switch (sizeof(*(ptr))) {			\
 	case 1: __get_user_asm(sr, val, "ldb", ptr); break; \
@@ -86,11 +83,12 @@ struct exception_table_entry {
 {							\
 	register long __gu_val;				\
 							\
-	__asm__("1: " ldx " 0(" sr "%2),%0\n"		\
+	__asm__("\tcopy %%r0,%1\n"			\
+		"1: " ldx " 0(" sr "%2),%0\n"		\
 		"9:\n"					\
 		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b)	\
-		: "=r"(__gu_val), "=r"(__gu_err)        \
-		: "r"(ptr), "1"(__gu_err));		\
+		: "=r"(__gu_val), "=&r"(__gu_err)	\
+		: "r"(ptr));				\
 							\
 	(val) = (__force __typeof__(*(ptr))) __gu_val;	\
 }
@@ -117,14 +115,15 @@ struct exception_table_entry {
 		__typeof__(*(ptr))	t;		\
 	} __gu_tmp;					\
 							\
-	__asm__("   copy %%r0,%R0\n"			\
+	__asm__("\tcopy %%r0,%R0\n"			\
+		"\tcopy %%r0,%1\n"			\
 		"1: ldw 0(" sr "%2),%0\n"		\
 		"2: ldw 4(" sr "%2),%R0\n"		\
 		"9:\n"					\
 		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b)	\
 		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, 9b)	\
-		: "=&r"(__gu_tmp.l), "=r"(__gu_err)	\
-		: "r"(ptr), "1"(__gu_err));		\
+		: "=&r"(__gu_tmp.l), "=&r"(__gu_err)	\
+		: "r"(ptr));				\
 							\
 	(val) = __gu_tmp.t;				\
 }
@@ -134,8 +133,8 @@ struct exception_table_entry {

 #define __put_user_internal(sr, x, ptr)				\
 ({								\
-	ASM_EXCEPTIONTABLE_VAR(__pu_err);		      	\
         __typeof__(*(ptr)) __x = (__typeof__(*(ptr)))(x);	\
+	register long __pu_err;					\
 								\
 	switch (sizeof(*(ptr))) {				\
 	case 1: __put_user_asm(sr, "stb", __x, ptr); break;	\
@@ -177,24 +176,26 @@ struct exception_table_entry {

 #define __put_user_asm(sr, stx, x, ptr)				\
 	__asm__ __volatile__ (					\
+		"\tcopy %%r0,%0\n"				\
 		"1: " stx " %2,0(" sr "%1)\n"			\
 		"9:\n"						\
 		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b)		\
-		: "=r"(__pu_err)				\
-		: "r"(ptr), "r"(x), "0"(__pu_err))
+		: "=&r"(__pu_err)				\
+		: "r"(ptr), "r"(x))


 #if !defined(CONFIG_64BIT)

 #define __put_user_asm64(sr, __val, ptr) do {			\
 	__asm__ __volatile__ (					\
+		"\tcopy %%r0,%0\n"				\
 		"1: stw %2,0(" sr "%1)\n"			\
 		"2: stw %R2,4(" sr "%1)\n"			\
 		"9:\n"						\
 		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b)		\
 		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, 9b)		\
-		: "=r"(__pu_err)				\
-		: "r"(ptr), "r"(__val), "0"(__pu_err));		\
+		: "=&r"(__pu_err)				\
+		: "r"(ptr), "r"(__val));			\
 } while (0)

 #endif /* !defined(CONFIG_64BIT) */
diff --git a/arch/parisc/mm/fault.c b/arch/parisc/mm/fault.c
index e9eabf8f14d7..ad9b5d843315 100644
--- a/arch/parisc/mm/fault.c
+++ b/arch/parisc/mm/fault.c
@@ -148,18 +148,38 @@ int fixup_exception(struct pt_regs *regs)
 		 * Fix up get_user() and put_user().
 		 * ASM_EXCEPTIONTABLE_ENTRY_EFAULT() sets the least-significant
 		 * bit in the relative address of the fixup routine to indicate
-		 * that gr[ASM_EXCEPTIONTABLE_REG] should be loaded with
-		 * -EFAULT to report a userspace access error.
+		 * that the error register should be loaded with -EFAULT to
+		 * report a userspace access error. The error register is zeroed
+		 * before doing the load or store, so the exception handler can
+		 * read the "copy %%r0,reg" instruction to extract the register.
 		 */
 		if (fix->fixup & 1) {
-			regs->gr[ASM_EXCEPTIONTABLE_REG] = -EFAULT;
+			u32 treg;

 			/* zero target register for get_user() */
 			if (parisc_acctyp(0, regs->iir) == VM_READ) {
-				int treg = regs->iir & 0x1f;
-				BUG_ON(treg == 0);
+				treg = regs->iir & 0x1f;
+				if (WARN_ON(treg == 0))
+					goto wrong_get_put_user;
 				regs->gr[treg] = 0;
 			}
+
+			/* check 1-2 previous assembly statement(s) */
+			__get_kernel_nofault(&treg, (regs->iaoq[0] - 4) & ~3,
+				u32, wrong_get_put_user);
+			if (!IS_ENABLED(CONFIG_64BIT) &&
+			    (treg & 0xffffff00) != 0x08000200)
+				__get_kernel_nofault(&treg,
+					(regs->iaoq[0] - 8) & ~3,
+					u32, wrong_get_put_user);
+			/* check assembly statement for: copy %r0,%rX */
+			if (WARN_ON((treg & 0xffffff00) != 0x08000200))
+				goto wrong_get_put_user;
+			/* extract error register used for get_user/put_user */
+			treg &= 0x1f;
+			if (WARN_ON(treg == 0))
+				goto wrong_get_put_user;
+			regs->gr[treg] = -EFAULT;
 		}

 		regs->iaoq[0] = (unsigned long)&fix->fixup + fix->fixup;
@@ -177,6 +197,7 @@ int fixup_exception(struct pt_regs *regs)
 		return 1;
 	}

+wrong_get_put_user:
 	return 0;
 }

--
2.34.1




[Index of Archives]     [Linux SoC]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux