Skip to contentSkip to conversation

__access_ok

DISCUSSION Carsten Langgaard 4 participants 6 messages Jun 17, 2002 – Jun 19, 2002 MIPS

lore
Share & tools
lore
#0 lore raw

The __access_ok macro in include/asm-mips64/uaccess.h need to be changed in order to work correctly, it's a copy from the 32-bit kernel. It's not good enough to simply check for the "sign bit" of the address. The area between USEG (XUSEG) and KSEG0 will in 64-bit addressing mode generate an address error, if
accessed. The size of the area depend on the number of virtual addressing bits, implemented in the CPU.

I have tried to come up with a patch, please see below. Any thought ? I also changed the macro in arch/mips64/kernel/unaligned.c

/Carsten

Index: include/asm-mips64//uaccess.h
===================================================================
RCS file:
/home/repository/sw/linux-2.4.18/include/asm-mips64/uaccess.h,v retrieving revision 1.1.1.1
diff -u -r1.1.1.1 uaccess.h
--- include/asm-mips64//uaccess.h 4 Mar 2002 11:13:26 -0000
1.1.1.1
+++ include/asm-mips64//uaccess.h 17 Jun 2002 11:35:32 -0000
@@ -12,6 +12,8 @@
#include <linux/errno.h>
#include <linux/sched.h>

+#include <asm/addrspace.h>
+
#define STR(x) __STR(x)
#define __STR(x) #x

@@ -40,16 +42,23 @@
* than tests.
*
* Address valid if:

- *  - "addr" doesn't have any high-bits set
- *  - AND "size" doesn't have any high-bits set
- *  - AND "addr+size" doesn't have any high-bits set
- *  - OR we are in kernel mode.
+ *  - In user mode and "addr" and "addr+size" in USEG (or XUSEG).
+ *  - OR we are in kernel mode and "addr" and "addr+size" isn't in the
+ *    area between USEG (XUSEG) and KSEG0.
  */
 #define
__ua_size(size)                                                       \
        (__builtin_constant_p(size) && (signed long) (size) > 0 ? 0 :

(size))

-#define __access_ok(addr,size,mask)
\
- (((signed long)((mask)&(addr | (addr + size) |
__ua_size(size)))) >= 0)
+static inline int
+__access_ok(unsigned long addr, unsigned long size, long mask) +{
+ if (((mask) && ((addr | (addr+size)) >= KUSIZE)) ||
+ (((addr | (addr+size)) < K0BASE) &&
+ ((addr | (addr+size)) >= KUSIZE)))
+ return 0;
+ else
+ return 1;
+}

#define __access_mask ((long)(get_fs().seg))

Index: arch/mips64/kernel/unaligned.c
===================================================================
RCS file:
/home/repository/sw/linux-2.4.18/arch/mips64/kernel/unaligned.c,v retrieving revision 1.2
diff -u -r1.2 unaligned.c
--- arch/mips64/kernel/unaligned.c 23 May 2002 11:11:45 -0000
1.2
+++ arch/mips64/kernel/unaligned.c 17 Jun 2002 11:51:30 -0000
@@ -89,11 +89,14 @@
#define __STR(x) #x

/*
- * User code may only access USEG; kernel code may access the
- * entire address space.
+ * User code may only access USEG;
+ * Kernel code may access the entire address space, except the area between
+ * USEG (XUSEG) and KSEG0.
*/
-#define check_axs(pc,a,s) \
- if ((long)(~(pc) & ((a) | ((a)+(s)))) < 0) \
+#define check_axs(pc,a,s)
\
+ if (((pc < KUSIZE) && (((a) | ((a)+(s))) >= KUSIZE))

||               \
+           ((((a) | ((a)+(s))) < K0BASE) &&
\
+            (((a) | ((a)+(s))) >= KUSIZE)))
\
                goto sigbus;

--

_    _ ____  ___   Carsten Langgaard   Mailto:[email protected]
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com
#1 lore raw
Quoted 8 lines · Carsten Langgaard

* Address valid if:

- *  - "addr" doesn't have any high-bits set
- *  - AND "size" doesn't have any high-bits set
- *  - AND "addr+size" doesn't have any high-bits set
- *  - OR we are in kernel mode.
+ *  - In user mode and "addr" and "addr+size" in USEG (or XUSEG).
+ *  - OR we are in kernel mode and "addr" and "addr+size" isn't in the

+ * area between USEG (XUSEG) and KSEG0.

You also need to test for high bit set in size. Otherwise, for example, if a process was ok to access range 0x40000000-0x40003fff, access_ok(0x40001000, 0xfffff100) would return 1. The addition will wrap around, leading to all sorts of fun havoc.

-Justin

#2 lore raw

Note, that this is in the 64-bit kernel, a size there the high bit is set, will be astronomical and not a true value.

/Carsten

Quoted 11 lines · Justin Carlson
Quoted 8 lines · Carsten Langgaard

* Address valid if:

- *  - "addr" doesn't have any high-bits set
- *  - AND "size" doesn't have any high-bits set
- *  - AND "addr+size" doesn't have any high-bits set
- *  - OR we are in kernel mode.
+ *  - In user mode and "addr" and "addr+size" in USEG (or XUSEG).
+ *  - OR we are in kernel mode and "addr" and "addr+size" isn't in the

+ * area between USEG (XUSEG) and KSEG0.

Quoted 6 lines

You also need to test for high bit set in size. Otherwise, for example, if a process was ok to access range 0x40000000-0x40003fff, access_ok(0x40001000, 0xfffff100) would return 1. The addition will wrap around, leading to all sorts of fun havoc.

-Justin

--

_    _ ____  ___   Carsten Langgaard   Mailto:[email protected]
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com
#3 lore raw
In reply to Carsten Langgaard

Just to be nit-picking, the end point should be (addr + size - 1).

Jun

Quoted 111 lines · Carsten Langgaard

The __access_ok macro in include/asm-mips64/uaccess.h need to be changed in order to work correctly, it's a copy from the 32-bit kernel. It's not good enough to simply check for the "sign bit" of the address. The area between USEG (XUSEG) and KSEG0 will in 64-bit addressing mode generate an address error, if
accessed. The size of the area depend on the number of virtual addressing bits, implemented in the CPU.

I have tried to come up with a patch, please see below. Any thought ? I also changed the macro in arch/mips64/kernel/unaligned.c

/Carsten

Index: include/asm-mips64//uaccess.h
===================================================================
RCS file:
/home/repository/sw/linux-2.4.18/include/asm-mips64/uaccess.h,v retrieving revision 1.1.1.1
diff -u -r1.1.1.1 uaccess.h
--- include/asm-mips64//uaccess.h 4 Mar 2002 11:13:26 -0000
1.1.1.1
+++ include/asm-mips64//uaccess.h 17 Jun 2002 11:35:32 -0000

(quoted diff) +5 -6
@@ -12,6 +12,8 @@
 #include <linux/errno.h>
 #include <linux/sched.h>
 
+#include <asm/addrspace.h>
+
 #define STR(x)  __STR(x)
 #define __STR(x)  #x
 
@@ -40,16 +42,23 @@
  * than tests.
  *
  * Address valid if:
- *  - "addr" doesn't have any high-bits set
- *  - AND "size" doesn't have any high-bits set
- *  - AND "addr+size" doesn't have any high-bits set
- *  - OR we are in kernel mode.
+ *  - In user mode and "addr" and "addr+size" in USEG (or XUSEG).
+ *  - OR we are in kernel mode and "addr" and "addr+size" isn't in the
+ *    area between USEG (XUSEG) and KSEG0.
  */
 #define
 __ua_size(size)                                                       \
        (__builtin_constant_p(size) && (signed long) (size) > 0 ? 0 :
 (size))
 
-#define __access_ok(addr,size,mask)
 \
-       (((signed long)((mask)&(addr | (addr + size) |

__ua_size(size)))) >= 0)

(quoted diff) +10 -0
+static inline int
+__access_ok(unsigned long addr, unsigned long size, long mask)
+{
+       if (((mask) && ((addr | (addr+size)) >= KUSIZE)) ||
+           (((addr | (addr+size)) < K0BASE) &&
+            ((addr | (addr+size)) >= KUSIZE)))
+               return 0;
+       else
+               return 1;
+}
 
 #define __access_mask ((long)(get_fs().seg))

Index: arch/mips64/kernel/unaligned.c
===================================================================
RCS file:
/home/repository/sw/linux-2.4.18/arch/mips64/kernel/unaligned.c,v retrieving revision 1.2
diff -u -r1.2 unaligned.c
--- arch/mips64/kernel/unaligned.c 23 May 2002 11:11:45 -0000
1.2
+++ arch/mips64/kernel/unaligned.c 17 Jun 2002 11:51:30 -0000

(quoted diff) +2 -2
@@ -89,11 +89,14 @@
 #define __STR(x)  #x
 
 /*
- * User code may only access USEG; kernel code may access the
- * entire address space.
+ * User code may only access USEG;
+ * Kernel code may access the entire address space, except the area

between
+ * USEG (XUSEG) and KSEG0.
*/

(quoted diff) +1 -2
-#define check_axs(pc,a,s)                              \
-       if ((long)(~(pc) & ((a) | ((a)+(s)))) < 0)      \
+#define check_axs(pc,a,s)

\
+ if (((pc < KUSIZE) && (((a) | ((a)+(s))) >= KUSIZE))

||               \
+           ((((a) | ((a)+(s))) < K0BASE) &&
\
+            (((a) | ((a)+(s))) >= KUSIZE)))
\
                goto sigbus;

--

_    _ ____  ___   Carsten Langgaard   Mailto:[email protected]
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com
#4 lore raw

That's true, lets do it right, thanks.

/Carsten

Quoted 107 lines · Jun Sun

Just to be nit-picking, the end point should be (addr + size - 1).

Jun

Quoted 101 lines · Carsten Langgaard

The __access_ok macro in include/asm-mips64/uaccess.h need to be changed in order to work correctly, it's a copy from the 32-bit kernel. It's not good enough to simply check for the "sign bit" of the address. The area between USEG (XUSEG) and KSEG0 will in 64-bit addressing mode generate an address error, if
accessed. The size of the area depend on the number of virtual addressing bits, implemented in the CPU.

I have tried to come up with a patch, please see below. Any thought ? I also changed the macro in arch/mips64/kernel/unaligned.c

/Carsten

Index: include/asm-mips64//uaccess.h
===================================================================
RCS file:
/home/repository/sw/linux-2.4.18/include/asm-mips64/uaccess.h,v retrieving revision 1.1.1.1
diff -u -r1.1.1.1 uaccess.h
--- include/asm-mips64//uaccess.h 4 Mar 2002 11:13:26 -0000
1.1.1.1
+++ include/asm-mips64//uaccess.h 17 Jun 2002 11:35:32 -0000

(quoted diff) +5 -6
@@ -12,6 +12,8 @@
 #include <linux/errno.h>
 #include <linux/sched.h>
 
+#include <asm/addrspace.h>
+
 #define STR(x)  __STR(x)
 #define __STR(x)  #x
 
@@ -40,16 +42,23 @@
  * than tests.
  *
  * Address valid if:
- *  - "addr" doesn't have any high-bits set
- *  - AND "size" doesn't have any high-bits set
- *  - AND "addr+size" doesn't have any high-bits set
- *  - OR we are in kernel mode.
+ *  - In user mode and "addr" and "addr+size" in USEG (or XUSEG).
+ *  - OR we are in kernel mode and "addr" and "addr+size" isn't in the
+ *    area between USEG (XUSEG) and KSEG0.
  */
 #define
 __ua_size(size)                                                       \
        (__builtin_constant_p(size) && (signed long) (size) > 0 ? 0 :
 (size))
 
-#define __access_ok(addr,size,mask)
 \
-       (((signed long)((mask)&(addr | (addr + size) |

__ua_size(size)))) >= 0)

(quoted diff) +10 -0
+static inline int
+__access_ok(unsigned long addr, unsigned long size, long mask)
+{
+       if (((mask) && ((addr | (addr+size)) >= KUSIZE)) ||
+           (((addr | (addr+size)) < K0BASE) &&
+            ((addr | (addr+size)) >= KUSIZE)))
+               return 0;
+       else
+               return 1;
+}
 
 #define __access_mask ((long)(get_fs().seg))

Index: arch/mips64/kernel/unaligned.c
===================================================================
RCS file:
/home/repository/sw/linux-2.4.18/arch/mips64/kernel/unaligned.c,v retrieving revision 1.2
diff -u -r1.2 unaligned.c
--- arch/mips64/kernel/unaligned.c 23 May 2002 11:11:45 -0000
1.2
+++ arch/mips64/kernel/unaligned.c 17 Jun 2002 11:51:30 -0000

(quoted diff) +2 -2
@@ -89,11 +89,14 @@
 #define __STR(x)  #x
 
 /*
- * User code may only access USEG; kernel code may access the
- * entire address space.
+ * User code may only access USEG;
+ * Kernel code may access the entire address space, except the area

between
+ * USEG (XUSEG) and KSEG0.
*/

(quoted diff) +1 -2
-#define check_axs(pc,a,s)                              \
-       if ((long)(~(pc) & ((a) | ((a)+(s)))) < 0)      \
+#define check_axs(pc,a,s)

\
+ if (((pc < KUSIZE) && (((a) | ((a)+(s))) >= KUSIZE))

||               \
+           ((((a) | ((a)+(s))) < K0BASE) &&
\
+            (((a) | ((a)+(s))) >= KUSIZE)))
\
                goto sigbus;
Quoted 10 lines
Quoted 10 lines

--

_    _ ____  ___   Carsten Langgaard  Mailto:[email protected]
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556

Denmark http://www.mips.com

--

_    _ ____  ___   Carsten Langgaard   Mailto:[email protected]
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com
#5 lore raw
In reply to Jun Sun
Quoted 1 line · Jun Sun

Just to be nit-picking, the end point should be (addr + size - 1).

You better don't use the last bit of userspace and make that (addr + size). Saves several kB on the entire kernel.

Ralf