Skip to content

Commit

Permalink
parisc: Clean up fixup routines for get_user()/put_user()
Browse files Browse the repository at this point in the history
Al Viro noticed that userspace accesses via get_user()/put_user() can be
simplified a lot with regard to usage of the exception handling.

This patch implements a fixup routine for get_user() and put_user() in such
that the exception handler will automatically load -EFAULT into the register
%r8 (the error value) in case on a fault on userspace.  Additionally the fixup
routine will zero the target register on fault in case of a get_user() call.
The target register is extracted out of the faulting assembly instruction.

This patch brings a few benefits over the old implementation:
1. Exception handling gets much cleaner, easier and smaller in size.
2. Helper functions like fixup_get_user_skip_1 (all of fixup.S) can be dropped.
3. No need to hardcode %r9 as target register for get_user() any longer. This
   helps the compiler register allocator and thus creates less assembler
   statements.
4. No dependency on the exception_data contents any longer.
5. Nested faults will be handled cleanly.

Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Cc: <stable@vger.kernel.org> # v4.9+
Signed-off-by: Helge Deller <deller@gmx.de>
  • Loading branch information
Helge Deller committed Mar 29, 2017
1 parent 554bfec commit d19f5e4
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 134 deletions.
59 changes: 34 additions & 25 deletions arch/parisc/include/asm/uaccess.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@ struct exception_table_entry {
".word (" #fault_addr " - .), (" #except_addr " - .)\n\t" \
".previous\n"

/*
* 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 %r8 for a read or write fault, and zeroes the target
* register in case of a read fault in get_user().
*/
#define ASM_EXCEPTIONTABLE_ENTRY_EFAULT( fault_addr, except_addr )\
ASM_EXCEPTIONTABLE_ENTRY( fault_addr, except_addr + 1)

/*
* The page fault handler stores, in a per-cpu area, the following information
* if a fixup routine is available.
Expand Down Expand Up @@ -91,7 +100,7 @@ struct exception_data {
#define __get_user(x, ptr) \
({ \
register long __gu_err __asm__ ("r8") = 0; \
register long __gu_val __asm__ ("r9") = 0; \
register long __gu_val; \
\
load_sr2(); \
switch (sizeof(*(ptr))) { \
Expand All @@ -107,22 +116,23 @@ struct exception_data {
})

#define __get_user_asm(ldx, ptr) \
__asm__("\n1:\t" ldx "\t0(%%sr2,%2),%0\n\t" \
ASM_EXCEPTIONTABLE_ENTRY(1b, fixup_get_user_skip_1)\
__asm__("1: " ldx " 0(%%sr2,%2),%0\n" \
"9:\n" \
ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b) \
: "=r"(__gu_val), "=r"(__gu_err) \
: "r"(ptr), "1"(__gu_err) \
: "r1");
: "r"(ptr), "1"(__gu_err));

#if !defined(CONFIG_64BIT)

#define __get_user_asm64(ptr) \
__asm__("\n1:\tldw 0(%%sr2,%2),%0" \
"\n2:\tldw 4(%%sr2,%2),%R0\n\t" \
ASM_EXCEPTIONTABLE_ENTRY(1b, fixup_get_user_skip_2)\
ASM_EXCEPTIONTABLE_ENTRY(2b, fixup_get_user_skip_1)\
__asm__(" copy %%r0,%R0\n" \
"1: ldw 0(%%sr2,%2),%0\n" \
"2: ldw 4(%%sr2,%2),%R0\n" \
"9:\n" \
ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b) \
ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, 9b) \
: "=r"(__gu_val), "=r"(__gu_err) \
: "r"(ptr), "1"(__gu_err) \
: "r1");
: "r"(ptr), "1"(__gu_err));

#endif /* !defined(CONFIG_64BIT) */

Expand All @@ -148,32 +158,31 @@ struct exception_data {
* The "__put_user/kernel_asm()" macros tell gcc they read from memory
* instead of writing. This is because they do not write to any memory
* gcc knows about, so there are no aliasing issues. These macros must
* also be aware that "fixup_put_user_skip_[12]" are executed in the
* context of the fault, and any registers used there must be listed
* as clobbers. In this case only "r1" is used by the current routines.
* r8/r9 are already listed as err/val.
* also be aware that fixups are executed in the context of the fault,
* and any registers used there must be listed as clobbers.
* r8 is already listed as err.
*/

#define __put_user_asm(stx, x, ptr) \
__asm__ __volatile__ ( \
"\n1:\t" stx "\t%2,0(%%sr2,%1)\n\t" \
ASM_EXCEPTIONTABLE_ENTRY(1b, fixup_put_user_skip_1)\
"1: " stx " %2,0(%%sr2,%1)\n" \
"9:\n" \
ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b) \
: "=r"(__pu_err) \
: "r"(ptr), "r"(x), "0"(__pu_err) \
: "r1")
: "r"(ptr), "r"(x), "0"(__pu_err))


#if !defined(CONFIG_64BIT)

#define __put_user_asm64(__val, ptr) do { \
__asm__ __volatile__ ( \
"\n1:\tstw %2,0(%%sr2,%1)" \
"\n2:\tstw %R2,4(%%sr2,%1)\n\t" \
ASM_EXCEPTIONTABLE_ENTRY(1b, fixup_put_user_skip_2)\
ASM_EXCEPTIONTABLE_ENTRY(2b, fixup_put_user_skip_1)\
"1: stw %2,0(%%sr2,%1)\n" \
"2: stw %R2,4(%%sr2,%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) \
: "r1"); \
: "r"(ptr), "r"(__val), "0"(__pu_err)); \
} while (0)

#endif /* !defined(CONFIG_64BIT) */
Expand Down
10 changes: 0 additions & 10 deletions arch/parisc/kernel/parisc_ksyms.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,6 @@ EXPORT_SYMBOL(__cmpxchg_u64);
EXPORT_SYMBOL(lclear_user);
EXPORT_SYMBOL(lstrnlen_user);

/* Global fixups - defined as int to avoid creation of function pointers */
extern int fixup_get_user_skip_1;
extern int fixup_get_user_skip_2;
extern int fixup_put_user_skip_1;
extern int fixup_put_user_skip_2;
EXPORT_SYMBOL(fixup_get_user_skip_1);
EXPORT_SYMBOL(fixup_get_user_skip_2);
EXPORT_SYMBOL(fixup_put_user_skip_1);
EXPORT_SYMBOL(fixup_put_user_skip_2);

#ifndef CONFIG_64BIT
/* Needed so insmod can set dp value */
extern int $global$;
Expand Down
2 changes: 1 addition & 1 deletion arch/parisc/lib/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Makefile for parisc-specific library files
#

lib-y := lusercopy.o bitops.o checksum.o io.o memset.o fixup.o memcpy.o \
lib-y := lusercopy.o bitops.o checksum.o io.o memset.o memcpy.o \
ucmpdi2.o delay.o

obj-y := iomap.o
98 changes: 0 additions & 98 deletions arch/parisc/lib/fixup.S

This file was deleted.

17 changes: 17 additions & 0 deletions arch/parisc/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,23 @@ int fixup_exception(struct pt_regs *regs)
d->fault_space = regs->isr;
d->fault_addr = regs->ior;

/*
* 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 %r8 should be loaded with -EFAULT to report a userspace
* access error.
*/
if (fix->fixup & 1) {
regs->gr[8] = -EFAULT;

/* zero target register for get_user() */
if (parisc_acctyp(0, regs->iir) == VM_READ) {
int treg = regs->iir & 0x1f;
regs->gr[treg] = 0;
}
}

regs->iaoq[0] = (unsigned long)&fix->fixup + fix->fixup;
regs->iaoq[0] &= ~3;
/*
Expand Down

0 comments on commit d19f5e4

Please sign in to comment.