* Arnd Bergmann <arnd@xxxxxxxxxx>:
> On Wed, Sep 8, 2021 at 11:40 PM Helge Deller <deller@xxxxxx> wrote:
> > On 9/8/21 11:26 PM, Arnd Bergmann wrote:
> > >> /*
> > >> * Complex access routines -- macros
> > >> */
> > >> -#define user_addr_max() (~0UL)
> > >> +#define user_addr_max() (uaccess_kernel() ? ~0UL : TASK_SIZE)
> >
> > I noticed that our user_addr_max() was actually wrong.
> > It's used in the generic strnlen_user() so fixing it seemed appropriate.
>
> The user_addr_max() definition should match what you do in access_ok()
> for USER_DS though, which is a nop on architectures that have split user
> address space, so I think the ~0UL was correct here.
>
> No matter what the arguments are, there is no danger of spilling over
> into kernel space, which is what the user_addr_max() check is trying
> to prevent on other architectures.
>
> > > We are getting very close to completely removing set_fs()/get_fs(),
> > > uaccess_kernel() and some related bits from the kernel, so I think
> > > it would be better to the other way here and finish off removing
> > > CONFIG_SET_FS from parisc.
> > >
> > > I think this will also simplify your asm/uaccess.h a lot, in particular
> > > since it has separate address spaces for __get_user() and
> > > __get_kernel_nofault(), and without set_fs() you can leave out
> > > the runtime conditional to switch between them.
> >
> > That's a good idea and should probably be done.
> > Do you have some pointers where to start, e.g. initial commits from other arches ?
>
> Russell just merged my series for arch/arm in linux-5.15, you
> can look at that but it's probably easier for parisc.
>
> I think the only part you need to add is __get_kernel_nofault()
> and __put_kernel_nofault(). You can see in mm/maccess.c
> what the difference between the two versions in there is.
>
> Once you have those, you define HAVE_GET_KERNEL_NOFAULT
> and then remove CONFIG_SET_FS, set_fs(), get_fs(), load_sr2(),
> thread_info->addr_limit, KERNEL_DS, and USER_DS.
Thanks for those hints!
The attached initial patch below seems to work, it boots into systemd.
It needs further cleanups though.
I wonder if you should add a workaround for 32-bit kernels
to not copy 64bit-wise, see my patch below in
copy_from_kernel_nofault():
- copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
+ /* avoid 64-bit accesses on 32-bit platforms */
+ if (sizeof(unsigned long) > 4)
+ copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
Helge
---
[PATCH] parisc: Implement __get/put_kernel_nofault()
Add __get_kernel_nofault() and __put_kernel_nofault(), define
HAVE_GET_KERNEL_NOFAULT and remove CONFIG_SET_FS, set_fs(), get_fs(),
load_sr2(), thread_info->addr_limit, KERNEL_DS, and USER_DS.
Signed-off-by: Helge Deller <deller@xxxxxx>
Suggested-by: Arnd Bergmann <arnd@xxxxxxxxxx>
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 3ae71994399c..b5bc346d464a 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -64,7 +64,6 @@ config PARISC
select HAVE_KPROBES_ON_FTRACE
select HAVE_DYNAMIC_FTRACE_WITH_REGS
select HAVE_SOFTIRQ_ON_OWN_STACK if IRQSTACKS
- select SET_FS
help
The PA-RISC microprocessor is designed by Hewlett-Packard and used
diff --git a/arch/parisc/include/asm/processor.h b/arch/parisc/include/asm/processor.h
index b5fbcd2c1780..eeb7da064289 100644
--- a/arch/parisc/include/asm/processor.h
+++ b/arch/parisc/include/asm/processor.h
@@ -101,10 +101,6 @@ DECLARE_PER_CPU(struct cpuinfo_parisc, cpu_data);
#define CPU_HVERSION ((boot_cpu_data.hversion >> 4) & 0x0FFF)
-typedef struct {
- int seg;
-} mm_segment_t;
-
#define ARCH_MIN_TASKALIGN 8
struct thread_struct {
diff --git a/arch/parisc/include/asm/thread_info.h b/arch/parisc/include/asm/thread_info.h
index 0bd38a972cea..00ad50fef769 100644
--- a/arch/parisc/include/asm/thread_info.h
+++ b/arch/parisc/include/asm/thread_info.h
@@ -11,7 +11,6 @@
struct thread_info {
struct task_struct *task; /* main task structure */
unsigned long flags; /* thread_info flags (see TIF_*) */
- mm_segment_t addr_limit; /* user-level address space limit */
__u32 cpu; /* current CPU */
int preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
};
@@ -21,7 +20,6 @@ struct thread_info {
.task = &tsk, \
.flags = 0, \
.cpu = 0, \
- .addr_limit = KERNEL_DS, \
.preempt_count = INIT_PREEMPT_COUNT, \
}
diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index a4368731f2b4..6e0ca7756ceb 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h
@@ -11,14 +11,6 @@
#include <linux/bug.h>
#include <linux/string.h>
-#define KERNEL_DS ((mm_segment_t){0})
-#define USER_DS ((mm_segment_t){1})
-
-#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
-
-#define get_fs() (current_thread_info()->addr_limit)
-#define set_fs(x) (current_thread_info()->addr_limit = (x))
-
/*
* Note that since kernel addresses are in a separate address space on
* parisc, we don't need to do anything for access_ok().
@@ -33,11 +25,11 @@
#define get_user __get_user
#if !defined(CONFIG_64BIT)
-#define LDD_USER(val, ptr) __get_user_asm64(val, ptr)
-#define STD_USER(x, ptr) __put_user_asm64(x, ptr)
+#define LDD_USER(sr, val, ptr, err_label) __get_user_asm64(sr, val, ptr, err_label)
+#define STD_USER(sr, x, ptr, err_label) __put_user_asm64(sr, x, ptr, err_label)
#else
-#define LDD_USER(val, ptr) __get_user_asm(val, "ldd", ptr)
-#define STD_USER(x, ptr) __put_user_asm("std", x, ptr)
+#define LDD_USER(sr, val, ptr, err_label) __get_user_asm(sr, val, "ldd", ptr, err_label)
+#define STD_USER(sr, x, ptr, err_label) __put_user_asm(sr, "std", x, ptr, err_label)
#endif
/*
@@ -67,28 +59,15 @@ struct exception_table_entry {
#define ASM_EXCEPTIONTABLE_ENTRY_EFAULT( fault_addr, except_addr )\
ASM_EXCEPTIONTABLE_ENTRY( fault_addr, except_addr + 1)
-/*
- * load_sr2() preloads the space register %%sr2 - based on the value of
- * get_fs() - with either a value of 0 to access kernel space (KERNEL_DS which
- * is 0), or with the current value of %%sr3 to access user space (USER_DS)
- * memory. The following __get_user_asm() and __put_user_asm() functions have
- * %%sr2 hard-coded to access the requested memory.
- */
-#define load_sr2() \
- __asm__(" or,= %0,%%r0,%%r0\n\t" \
- " mfsp %%sr3,%0\n\t" \
- " mtsp %0,%%sr2\n\t" \
- : : "r"(get_fs()) : )
-
-#define __get_user_internal(val, ptr) \
+#define __get_user_internal(sr, val, ptr, err_label) \
({ \
register long __gu_err __asm__ ("r8") = 0; \
\
switch (sizeof(*(ptr))) { \
- case 1: __get_user_asm(val, "ldb", ptr); break; \
- case 2: __get_user_asm(val, "ldh", ptr); break; \
- case 4: __get_user_asm(val, "ldw", ptr); break; \
- case 8: LDD_USER(val, ptr); break; \
+ case 1: __get_user_asm(sr, val, "ldb", ptr, err_label); break; \
+ case 2: __get_user_asm(sr, val, "ldh", ptr, err_label); break; \
+ case 4: __get_user_asm(sr, val, "ldw", ptr, err_label); break; \
+ case 8: LDD_USER(sr, val, ptr, err_label); break; \
default: BUILD_BUG(); \
} \
\
@@ -97,26 +76,38 @@ struct exception_table_entry {
#define __get_user(val, ptr) \
({ \
- load_sr2(); \
- __get_user_internal(val, ptr); \
+ __get_user_internal("%%sr3,", val, ptr, 9b); \
})
-#define __get_user_asm(val, ldx, ptr) \
+#define __get_user_asm(sr, val, ldx, ptr, err_label) \
{ \
register long __gu_val; \
\
- __asm__("1: " ldx " 0(%%sr2,%2),%0\n" \
+ __asm__("1: " ldx " 0(" sr "%2),%0\n" \
"9:\n" \
- ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b) \
+ ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, err_label) \
: "=r"(__gu_val), "=r"(__gu_err) \
: "r"(ptr), "1"(__gu_err)); \
\
(val) = (__force __typeof__(*(ptr))) __gu_val; \
}
+#define HAVE_GET_KERNEL_NOFAULT
+#define __get_kernel_nofault(dst, src, type, err_label) \
+{ \
+ type __z; \
+ long __err; \
+ __err = __get_user_internal("", __z, (type *)(src), 9b); \
+ if (unlikely(__err)) \
+ goto err_label; \
+ else \
+ *(type *)(dst) = __z; \
+}
+
+
#if !defined(CONFIG_64BIT)
-#define __get_user_asm64(val, ptr) \
+#define __get_user_asm64(sr, val, ptr, err_label) \
{ \
union { \
unsigned long long l; \
@@ -124,11 +115,11 @@ struct exception_table_entry {
} __gu_tmp; \
\
__asm__(" copy %%r0,%R0\n" \
- "1: ldw 0(%%sr2,%2),%0\n" \
- "2: ldw 4(%%sr2,%2),%R0\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) \
+ ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, err_label) \
+ ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, err_label) \
: "=&r"(__gu_tmp.l), "=r"(__gu_err) \
: "r"(ptr), "1"(__gu_err)); \
\
@@ -138,16 +129,16 @@ struct exception_table_entry {
#endif /* !defined(CONFIG_64BIT) */
-#define __put_user_internal(x, ptr) \
+#define __put_user_internal(sr, x, ptr, err_label) \
({ \
register long __pu_err __asm__ ("r8") = 0; \
__typeof__(*(ptr)) __x = (__typeof__(*(ptr)))(x); \
\
switch (sizeof(*(ptr))) { \
- case 1: __put_user_asm("stb", __x, ptr); break; \
- case 2: __put_user_asm("sth", __x, ptr); break; \
- case 4: __put_user_asm("stw", __x, ptr); break; \
- case 8: STD_USER(__x, ptr); break; \
+ case 1: __put_user_asm(sr, "stb", __x, ptr, err_label); break; \
+ case 2: __put_user_asm(sr, "sth", __x, ptr, err_label); break; \
+ case 4: __put_user_asm(sr, "stw", __x, ptr, err_label); break; \
+ case 8: STD_USER(sr, __x, ptr, err_label); break; \
default: BUILD_BUG(); \
} \
\
@@ -156,10 +147,20 @@ struct exception_table_entry {
#define __put_user(x, ptr) \
({ \
- load_sr2(); \
- __put_user_internal(x, ptr); \
+ __put_user_internal("%%sr3,", x, ptr, 9b); \
})
+#define __put_kernel_nofault(dst, src, type, err_label) \
+{ \
+ type __z = *(type *)(src); \
+ long __err; \
+ __err = __put_user_internal("", __z, (type *)(dst), 9b);\
+ if (unlikely(__err)) \
+ goto err_label; \
+}
+
+
+
/*
* The "__put_user/kernel_asm()" macros tell gcc they read from memory
@@ -170,26 +171,26 @@ struct exception_table_entry {
* r8 is already listed as err.
*/
-#define __put_user_asm(stx, x, ptr) \
- __asm__ __volatile__ ( \
- "1: " stx " %2,0(%%sr2,%1)\n" \
- "9:\n" \
- ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b) \
- : "=r"(__pu_err) \
+#define __put_user_asm(sr, stx, x, ptr, err_label) \
+ __asm__ __volatile__ ( \
+ "1: " stx " %2,0(" sr "%1)\n" \
+ "9:\n" \
+ ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, err_label) \
+ : "=r"(__pu_err) \
: "r"(ptr), "r"(x), "0"(__pu_err))
#if !defined(CONFIG_64BIT)
-#define __put_user_asm64(__val, ptr) do { \
- __asm__ __volatile__ ( \
- "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)); \
+#define __put_user_asm64(sr, __val, ptr, err_label) do { \
+ __asm__ __volatile__ ( \
+ "1: stw %2,0(" sr "%1)\n" \
+ "2: stw %R2,4(" sr "%1)\n" \
+ "9:\n" \
+ ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, err_label) \
+ ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, err_label) \
+ : "=r"(__pu_err) \
+ : "r"(ptr), "r"(__val), "0"(__pu_err)); \
} while (0)
#endif /* !defined(CONFIG_64BIT) */
@@ -205,7 +206,6 @@ extern __must_check long strnlen_user(const char __user *src, long n);
/*
* Complex access routines -- macros
*/
-#define user_addr_max() (~0UL)
#define clear_user lclear_user
#define __clear_user lclear_user
diff --git a/arch/parisc/kernel/asm-offsets.c b/arch/parisc/kernel/asm-offsets.c
index 33113ba24054..7c83915e4aed 100644
--- a/arch/parisc/kernel/asm-offsets.c
+++ b/arch/parisc/kernel/asm-offsets.c
@@ -230,7 +230,7 @@ int main(void)
DEFINE(TI_TASK, offsetof(struct thread_info, task));
DEFINE(TI_FLAGS, offsetof(struct thread_info, flags));
DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
- DEFINE(TI_SEGMENT, offsetof(struct thread_info, addr_limit));
+// DEFINE(TI_SEGMENT, offsetof(struct thread_info, addr_limit));
DEFINE(TI_PRE_COUNT, offsetof(struct thread_info, preempt_count));
DEFINE(THREAD_SZ, sizeof(struct thread_info));
/* THREAD_SZ_ALGN includes space for a stack frame. */
diff --git a/arch/parisc/lib/lusercopy.S b/arch/parisc/lib/lusercopy.S
index 0aad5ce89f4d..29eafd963ac0 100644
--- a/arch/parisc/lib/lusercopy.S
+++ b/arch/parisc/lib/lusercopy.S
@@ -34,11 +34,11 @@
*/
.macro get_sr
- mfctl %cr30,%r1
- ldw TI_SEGMENT(%r1),%r22
+// mfctl %cr30,%r1
+// ldw TI_SEGMENT(%r1),%r22
mfsp %sr3,%r1
- or,<> %r22,%r0,%r0
- copy %r0,%r1
+// or,<> %r22,%r0,%r0
+// copy %r0,%r1
mtsp %r1,%sr1
.endm
diff --git a/mm/maccess.c b/mm/maccess.c
index 3bd70405f2d8..85eb86d6cab4 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -28,7 +28,9 @@ long copy_from_kernel_nofault(void *dst, const void *src, size_t size)
return -ERANGE;
pagefault_disable();
- copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
+ /* avoid 64-bit accesses on 32-bit platforms */
+ if (sizeof(unsigned long) > 4)
+ copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
copy_from_kernel_nofault_loop(dst, src, size, u32, Efault);
copy_from_kernel_nofault_loop(dst, src, size, u16, Efault);
copy_from_kernel_nofault_loop(dst, src, size, u8, Efault);
@@ -51,7 +53,9 @@ EXPORT_SYMBOL_GPL(copy_from_kernel_nofault);
long copy_to_kernel_nofault(void *dst, const void *src, size_t size)
{
pagefault_disable();
- copy_to_kernel_nofault_loop(dst, src, size, u64, Efault);
+ /* avoid 64-bit accesses on 32-bit platforms */
+ if (sizeof(unsigned long) > 4)
+ copy_to_kernel_nofault_loop(dst, src, size, u64, Efault);
copy_to_kernel_nofault_loop(dst, src, size, u32, Efault);
copy_to_kernel_nofault_loop(dst, src, size, u16, Efault);
copy_to_kernel_nofault_loop(dst, src, size, u8, Efault);
