__access_ok

__access_ok

[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
@@ -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:carstenl@mips.com
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com

Re: __access_ok

[Justin Carlson]

On Mon, 2002-06-17 at 04:53, Carsten Langgaard wrote:

>   * 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

Re: __access_ok

[Jun Sun]

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

Jun

Carsten Langgaard wrote:

> 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:carstenl@mips.com
> |\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
> | \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
>   TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
>                    Denmark             http://www.mips.com
> 
> 
> 
> 

Re: __access_ok

[Carsten Langgaard]

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

/Carsten


Jun Sun wrote:

> Just to be nit-picking, the end point should be (addr + size - 1).
>
> Jun
>
> Carsten Langgaard wrote:
>
> > 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:carstenl@mips.com
> > |\  /|||___)(___   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:carstenl@mips.com
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com

Re: __access_ok

[Carsten Langgaard]

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

Justin Carlson wrote:

> On Mon, 2002-06-17 at 04:53, Carsten Langgaard wrote:
>
> >   * 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

--
_    _ ____  ___   Carsten Langgaard   Mailto:carstenl@mips.com
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com

Re: __access_ok

[Ralf Baechle]

On Mon, Jun 17, 2002 at 12:55:08PM -0700, Jun Sun wrote:

> 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