2022-02-07 17:25:00

by Stafford Horne

[permalink] [raw]
Subject: [PATCH] openrisc: remove CONFIG_SET_FS

Remove the address space override API set_fs() used for User Mode Linux.
User address space is now limited to TASK_SIZE.

To support this we implement and wire in __get_kernel_nofault and
__set_kernel_nofault.

The function user_addr_max is removed as there is a default definition
provided when CONFIG_SET_FS is not used.

Signed-off-by: Stafford Horne <[email protected]>
---
arch/openrisc/Kconfig | 1 -
arch/openrisc/include/asm/thread_info.h | 7 ----
arch/openrisc/include/asm/uaccess.h | 48 +++++++++++--------------
3 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index bf047dca7ec6..ceda77fb8bc8 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -36,7 +36,6 @@ config OPENRISC
select ARCH_WANT_FRAME_POINTERS
select GENERIC_IRQ_MULTI_HANDLER
select MMU_GATHER_NO_RANGE if MMU
- select SET_FS
select TRACE_IRQFLAGS_SUPPORT

config CPU_BIG_ENDIAN
diff --git a/arch/openrisc/include/asm/thread_info.h b/arch/openrisc/include/asm/thread_info.h
index 659834ab87fa..4af3049c34c2 100644
--- a/arch/openrisc/include/asm/thread_info.h
+++ b/arch/openrisc/include/asm/thread_info.h
@@ -40,18 +40,12 @@
*/
#ifndef __ASSEMBLY__

-typedef unsigned long mm_segment_t;
-
struct thread_info {
struct task_struct *task; /* main task structure */
unsigned long flags; /* low level flags */
__u32 cpu; /* current CPU */
__s32 preempt_count; /* 0 => preemptable, <0 => BUG */

- mm_segment_t addr_limit; /* thread address space:
- 0-0x7FFFFFFF for user-thead
- 0-0xFFFFFFFF for kernel-thread
- */
__u8 supervisor_stack[0];

/* saved context data */
@@ -71,7 +65,6 @@ struct thread_info {
.flags = 0, \
.cpu = 0, \
.preempt_count = INIT_PREEMPT_COUNT, \
- .addr_limit = KERNEL_DS, \
.ksp = 0, \
}

diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index 120f5005461b..cc9c5d8fd183 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -23,36 +23,12 @@
#include <asm/page.h>
#include <asm/extable.h>

-/*
- * The fs value determines whether argument validity checking should be
- * performed or not. If get_fs() == USER_DS, checking is performed, with
- * get_fs() == KERNEL_DS, checking is bypassed.
- *
- * For historical reasons, these macros are grossly misnamed.
- */
-
-/* addr_limit is the maximum accessible address for the task. we misuse
- * the KERNEL_DS and USER_DS values to both assign and compare the
- * addr_limit values through the equally misnamed get/set_fs macros.
- * (see above)
- */
-
-#define KERNEL_DS (~0UL)
-
-#define USER_DS (TASK_SIZE)
-#define get_fs() (current_thread_info()->addr_limit)
-#define set_fs(x) (current_thread_info()->addr_limit = (x))
-
-#define uaccess_kernel() (get_fs() == KERNEL_DS)
-
/* Ensure that the range from addr to addr+size is all within the process'
* address space
*/
static inline int __range_ok(unsigned long addr, unsigned long size)
{
- const mm_segment_t fs = get_fs();
-
- return size <= fs && addr <= (fs - size);
+ return size <= TASK_SIZE && addr <= (TASK_SIZE - size);
}

#define access_ok(addr, size) \
@@ -241,6 +217,25 @@ do { \
(__typeof__((x)-(x)))__gu_tmp); \
}

+#define __get_kernel_nofault(dst, src, type, label) \
+{ \
+ type __user *p = (type __force __user *)(src); \
+ type data; \
+ if (__get_user(data, p)) \
+ goto label; \
+ *(type *)dst = data; \
+}
+
+#define __put_kernel_nofault(dst, src, type, label) \
+{ \
+ type __user *p = (type __force __user *)(dst); \
+ type data = *(type *)src; \
+ if (__put_user(data, p)) \
+ goto label; \
+}
+
+#define HAVE_GET_KERNEL_NOFAULT
+
/* more complex routines */

extern unsigned long __must_check
@@ -268,9 +263,6 @@ clear_user(void __user *addr, unsigned long size)
return size;
}

-#define user_addr_max() \
- (uaccess_kernel() ? ~0UL : TASK_SIZE)
-
extern long strncpy_from_user(char *dest, const char __user *src, long count);

extern __must_check long strnlen_user(const char __user *str, long n);
--
2.31.1



2022-02-08 17:46:29

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] openrisc: remove CONFIG_SET_FS

On Sun, Feb 06, 2022 at 10:36:47AM +0900, Stafford Horne wrote:
> Remove the address space override API set_fs() used for User Mode Linux.

This ain't UML :)

> + return size <= TASK_SIZE && addr <= (TASK_SIZE - size);

No need for the inner braces.

2022-02-09 08:57:45

by Stafford Horne

[permalink] [raw]
Subject: Re: [PATCH] openrisc: remove CONFIG_SET_FS

On Mon, Feb 07, 2022 at 05:14:43PM +0000, David Laight wrote:
> From: Christoph Hellwig
> > Sent: 07 February 2022 06:45
> >
> > On Sun, Feb 06, 2022 at 10:36:47AM +0900, Stafford Horne wrote:
> > > Remove the address space override API set_fs() used for User Mode Linux.
> >
> > This ain't UML :)
> >
> > > + return size <= TASK_SIZE && addr <= (TASK_SIZE - size);
> >
> > No need for the inner braces.
>
> Since TASK_SIZE is actually an address wouldn't be better to
> swap the condition around (in every architecture).
>
> return addr <= TASK_SIZE && size <= TASK_SIZE - addr;

Hi David,

I like that, it is more clear, I will update to that in v3.

-Stafford

2022-02-09 11:47:44

by Stafford Horne

[permalink] [raw]
Subject: Re: [PATCH] openrisc: remove CONFIG_SET_FS

On Sun, Feb 06, 2022 at 10:45:06PM -0800, Christoph Hellwig wrote:
> On Sun, Feb 06, 2022 at 10:36:47AM +0900, Stafford Horne wrote:
> > Remove the address space override API set_fs() used for User Mode Linux.
>
> This ain't UML :)

Yes, Geert also brought that up. As I mentioned there I took the text from your
commit message in commit 8bb227ac34c0 ("um: remove set_fs").

I didn't realize arch/um is for User Model Linux, I always thought um was just
some other processor! Next, I thought your comment 'used for User Mode Linux'
was just referring to userpsace.

Now I get it!

I will fix this up in v2.

> > + return size <= TASK_SIZE && addr <= (TASK_SIZE - size);
>
> No need for the inner braces.

You mean to just write as:

return size <= TASK_SIZE && addr <= TASK_SIZE - size;

I prefer keeping the braces around (TASK_SIZE - size).

-Stafford

2022-02-09 12:28:46

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] openrisc: remove CONFIG_SET_FS

From: Christoph Hellwig
> Sent: 07 February 2022 06:45
>
> On Sun, Feb 06, 2022 at 10:36:47AM +0900, Stafford Horne wrote:
> > Remove the address space override API set_fs() used for User Mode Linux.
>
> This ain't UML :)
>
> > + return size <= TASK_SIZE && addr <= (TASK_SIZE - size);
>
> No need for the inner braces.

Since TASK_SIZE is actually an address wouldn't be better to
swap the condition around (in every architecture).

return addr <= TASK_SIZE && size <= TASK_SIZE - addr;

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)