A question about PROT_NONE on ARM and ARM26

A question about PROT_NONE on ARM and ARM26

[Jamie Lokier]

Hi folks,

I'm doing a survey of the different architectural implementations of
PROT_* flags for mmap() and mprotect().  I'm looking at linux-2.6.5.

The ARM and ARM26 implementations are very similar to plain x86: read
implies exec, exec implies read and write implies read.

But I see a potential bug with PROT_NONE.  I'm not sure if it's real,
so could you please confirm?

In include/asm-arm26/pgtable.h, I see this (reindented for mail):

#define PAGE_NONE \
       __pgprot(_PAGE_PRESENT | _PAGE_CLEAN | _PAGE_READONLY | _PAGE_NOT_USER)
#define PAGE_READONLY \
       __pgprot(_PAGE_PRESENT | _PAGE_CLEAN | _PAGE_READONLY                 )

In include/asm-arm/pgtable.h, I see this (reindented for mail):

#define _L_PTE_DEFAULT \
        L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_CACHEABLE | L_PTE_BUFFERABLE
#define _L_PTE_READ \
        L_PTE_USER | L_PTE_EXEC
#define PAGE_NONE \
        __pgprot(_L_PTE_DEFAULT)
#define PAGE_READONLY
        __pgprot(_L_PTE_DEFAULT | _L_PTE_READ)

Apparently the difference between PAGE_NONE and PAGE_READONLY, in each
case, is that PAGE_NONE is not readable from userspace but _is_
readable from kernel space.

Therefore all user accesses to a PROT_NONE page will cause a fault.

My question is: if the _kernel_ reads a PROT_NONE page, will it fault?
It looks likely to me.

This means that calling write() with a PROT_NONE region would succeed,
wouldn't it?

If so, this is a bug.  A minor bug, perhaps, but nonetheless I wish to
document it.

I don't know if you would be able to rearrange the pte bits so that a
PROT_NONE page is not accessible to the kernel either.  E.g. on i386
this is done by making PROT_NONE not set the hardware's present bit
but a different bit, and "pte_present()" tests both of those bits to
test the virtual present bit.

Thanks,
-- Jamie

Re: A question about PROT_NONE on ARM and ARM26

[William Lee Irwin III]

On Wed, Jun 30, 2004 at 03:44:34AM +0100, Jamie Lokier wrote:
> Apparently the difference between PAGE_NONE and PAGE_READONLY, in each
> case, is that PAGE_NONE is not readable from userspace but _is_
> readable from kernel space.
> Therefore all user accesses to a PROT_NONE page will cause a fault.
> My question is: if the _kernel_ reads a PROT_NONE page, will it fault?
> It looks likely to me.
> This means that calling write() with a PROT_NONE region would succeed,
> wouldn't it?
> If so, this is a bug.  A minor bug, perhaps, but nonetheless I wish to
> document it.
> I don't know if you would be able to rearrange the pte bits so that a
> PROT_NONE page is not accessible to the kernel either.  E.g. on i386
> this is done by making PROT_NONE not set the hardware's present bit
> but a different bit, and "pte_present()" tests both of those bits to
> test the virtual present bit.

It would be a bug if copy_to_user()/copy_from_user() failed to return
errors on attempted copies to/from areas with PROT_NONE protection.

I recommend writing a testcase and submitting it to LTP. I'll follow up
with an additional suggestion.

-- wli

Testing PROT_NONE and other protections, and a surprise

[Jamie Lokier]

William Lee Irwin III wrote:
> It would be a bug if copy_to_user()/copy_from_user() failed to return
> errors on attempted copies to/from areas with PROT_NONE protection.
> 
> I recommend writing a testcase and submitting it to LTP. I'll follow up
> with an additional suggestion.

I've just written a thorough test.  The attached program tries every
combination of PROT_* flags, and tells you what protection you really get.
I don't know how tests get into LTP; perhaps I can leave that to you?

When running it on i386, I got a *huge* surprise (to me).  A
PROT_WRITE-only page can sometimes fault on read or exec.  This is the
output:

Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX
========================================================================
MAP_SHARED     | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx
MAP_PRIVATE    | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx

The "!" means that a read or exec *sometimes* raises a signal.

(In general you cannot predict when it will or won't, because that can
depends on background paging decisions.)

Now, this makes complete sense when you think about how the page fault
path works.  But it's quite surprising behaviour from an application
point of view.  It's widely said that "PROT_WRITE implies PROT_READ on
i386" (and in fact on all architectures except IA64).  This shows that
it isn't quite true.

This program should hopefully run on all architectures, however it
does depend on an empty function working when relocated.  That might
not be the case.  A file is written and then mapped, to ensure that
i-cache coherency isn't a problem.

It'll be interesting to see the results on other architectures.

-- Jamie

==== test_mmap_prot.c ====
/* Test actual page permissions for PROT_* combinations with mmap()

   Copyright (C) 2004  Jamie Lokier

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software
   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */


#include <unistd.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/types.h>
#include <stdio.h>
#include <setjmp.h>
#include <sys/signal.h>

static sigjmp_buf buf;
static int fd;
static size_t size;

/* Hopefully this function is so simple it executes correctly even
   when relocated. */
void void_function (void) {}
void void_function_end (void) {}

void sigsegv (int sig)
{
	siglongjmp (buf, 1);
}

void setup (void)
{
	char * buffer;
	size = getpagesize ();
	buffer = calloc (1, size); /* All zeros. */
	if (!buffer) {
		perror ("calloc");
		exit (1);
	}
	fd = open ("/tmp/test_mmap_file", O_CREAT | O_TRUNC | O_RDWR, 0666);
	if (fd == -1) {
		perror ("open");
		exit (1);
	}
	if (unlink ("/tmp/test_mmap_file") != 0) {
		perror ("unlink");
		exit (1);
	}
	memcpy (buffer, (const char *) &void_function,
		((const char *) &void_function_end
		 - (const char *) &void_function));
	if (write (fd, buffer, size) != size) {
		perror ("write");
		exit (1);
	}
	signal (SIGSEGV, sigsegv);
	signal (SIGBUS, sigsegv); /* For those that don't use SIGSEGV... */
}

char * map (int private, int r, int w, int x)
{
	char * addr = mmap (0, size, ((r ? PROT_READ : 0)
				      | (w ? PROT_WRITE : 0)
				      | (x ? PROT_EXEC : 0)),
			    MAP_FILE | (private ? MAP_PRIVATE : MAP_SHARED),
			    fd, 0);
	if (addr == (char *) MAP_FAILED) {
		perror ("mmap");
		exit (1);
	}
	return addr;
}

void test (int private, int R, int W, int X)
{
	int i, dummy;
	char * addr = map (private, R, W, X);
	int r = 0, w = 0, x = 0;

	for (i = 0; i < 10; i++) {
		/* Test read. */
		if (!sigsetjmp (buf, 1)) {
			dummy = *(volatile char *) addr;
			r++;
		}

		/* Ensure page is fresh, if necessary. */
		if (i == 0) {
			munmap (addr, size);
			addr = map (private, R, W, X);
		}

		/* Test write. */
		if (!sigsetjmp (buf, 1)) {
			/* Don't clobber the executable code. */
			*((volatile char *) (addr + size - 1)) = 1;
			w++;
		}
			    
		/* Ensure page is fresh, if necessary. */
		if (i == 0) {
			munmap (addr, size);
			addr = map (private, R, W, X);
		}

		/* Test exec. */
		if (!sigsetjmp (buf, 1)) {
			((void (*) (void)) addr) ();
			x++;
		}
	}
	munmap (addr, size);

	printf ("%c%c%c", (r == 10 ? 'r' : r == 0 ? '-' : '!'),
		(w == 10 ? 'w' : w == 0 ? '-' : '!'),
		(x == 10 ? 'x' : x == 0 ? '-' : '!'));
}

int main ()
{
	int private, R, W, X;

	setup ();
	printf ("Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX\n"
		"========================================================================\n");

	for (private = 0; private <= 1; private++) {
		printf ("%s    | ", private ? "MAP_PRIVATE" : "MAP_SHARED ");

		for (X = 0; X <= 1; X++) {
			for (W = 0; W <= 1; W++) {
				for (R = 0; R <= 1; R++) {
					if (R | W | X)
						printf ("    ");
					test (private, R, W, X);
				}
			}
		}

		printf ("\n");
	}
	return 0;
}

Re: Testing PROT_NONE and other protections, and a surprise

[William Lee Irwin III]

William Lee Irwin III wrote:
>> It would be a bug if copy_to_user()/copy_from_user() failed to return
>> errors on attempted copies to/from areas with PROT_NONE protection.
>> I recommend writing a testcase and submitting it to LTP. I'll follow up
>> with an additional suggestion.

On Thu, Jul 01, 2004 at 04:26:06AM +0100, Jamie Lokier wrote:
> I've just written a thorough test.  The attached program tries every
> combination of PROT_* flags, and tells you what protection you really get.
> I don't know how tests get into LTP; perhaps I can leave that to you?
> When running it on i386, I got a *huge* surprise (to me).  A
> PROT_WRITE-only page can sometimes fault on read or exec.  This is the
> output:

This is unsurprising. The permissions can't be represented in pagetables,
but can opportunistically be enforced when exceptions are taken for other
reasons (e.g. TLB invalidations related to page replacement).


-- wli

Re: Testing PROT_NONE and other protections, and a surprise

[Jamie Lokier]

William Lee Irwin III wrote:
> > When running it on i386, I got a *huge* surprise (to me).  A
> > PROT_WRITE-only page can sometimes fault on read or exec.  This is the
> > output:
> 
> This is unsurprising. The permissions can't be represented in pagetables,
> but can opportunistically be enforced when exceptions are taken for other
> reasons (e.g. TLB invalidations related to page replacement).

Sure, it makes sense.  I understand exactly why the results are like that.
But it was a surprise because it just hadn't occurred to me.

The big point is that I know other people haven't thought of it
either.  I'm sure it's commonly thought that when you specify
PROT_WRITE, you'll get a readable page on architectures that can't
do write-only pages.

Interestingly, on Alpha you _will_ always get a readable page.
The Alpha fault handler has different enforcement rules to i386.

I know why this is done too, but it does indicate that the actual
behaviour of different architectures isn't immediately obvious.

The lessons from this investigation for portable code are:

    1. You really must specify all the access protections you need,
       i.e. read, write and/or exec if you use them.  This was obvious.

    2. In general you cannot depend on a kernel to _prevent_ accesses
       of a certain kind if you don't enable that in the protection flags,
       unless you know that architecture and its kernel well.
       This was obvious too.

    3. Less obviously, if you find that turning off a flag appears to
       prevent access of that type in tests, that is _not_ evidence
       that it will always prevent access of that type, even on the
       same machine.

       This is important if you're hoping to prevent certain accesses
       and trap them, e.g. for a garbage collector, virtual machine,
       memory access checker etc.  It has implications for
       autoconf-style tests which check for these properties.

       There are two exceptions which you can depend on across all
       architectures, at least running Linux:

    4. Exception #1: Turning off PROT_WRITE always disables writing.
       This is probably portable to every system which supports mprotect().

    5. Exception #2: PROT_NONE works.  Any access will fail, and you
       can trap the signal.  Historically this hasn't always been the
       case with Linux, though.  It is probably much less dependable
       among other operating systems than exception #1.

People knew these rules already.  It's implicit in a lot of code.
However I've not seen them clearly written down in one place.
Now they are.

Feel free to add any I missed, or provide corrections, thanks!

-- Jamie

Re: Testing PROT_NONE and other protections, and a surprise

[Kyle Moffett]

On Jun 30, 2004, at 23:26, Jamie Lokier wrote:
### i386 ###
> Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX
> ======================================================================= 
> =
> MAP_SHARED     | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx
> MAP_PRIVATE    | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx

### ppc32 ### Appears to be the same
Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX
========================================================================
MAP_SHARED     | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx
MAP_PRIVATE    | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx

Just for kicks, I ran this on Mac OS X too :-D  Interesting results!
Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX
========================================================================
MAP_SHARED     | ---    r-x    ---    rwx    ---    r-x    ---    rwx
MAP_PRIVATE    | ---    r-x    ---    rwx    ---    r-x    ---    rwx

Cheers,
Kyle Moffett

Re: Testing PROT_NONE and other protections, and a surprise

[Jamie Lokier]

Kyle Moffett wrote:
> Just for kicks, I ran this on Mac OS X too :-D  Interesting results!
> Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX
> ========================================================================
> MAP_SHARED     | ---    r-x    ---    rwx    ---    r-x    ---    rwx
> MAP_PRIVATE    | ---    r-x    ---    rwx    ---    r-x    ---    rwx

Yikes.  I wonder if those results are correct.

To be honest, if those results are correct it looks like a MacOS X
bug, or at least POSIX non-conformance.  It should always grant a
superset of the requested protections.

That invalidates the portability rule "ask for the permissions you
need to use".  If you only need to write or execute a file, and you
only ask for those, MacOS X won't let you.  So the rule needs to be
"and always include PROT_READ in the list".  Assuming it's not a bug
in the test program.

By the way, my program is potentially slightly flaky on architectures
where the CPU can't do byte writes (such as old Alphas).  Depending on
the OS, the program might say write access isn't granted for a
write-only request, when it is.  The program should've done a word
write instead.

I doubt that is the cause of those results on a PPC running MacOS X though :)

Can you confirm in a simple way that mapping a file, or some anonymous
memory, without PROT_READ, really isn't writable under MacOS X?  Can
you confirm it with a word write, if that would be relevant?

Cheers,
-- Jamie

Re: Testing PROT_NONE and other protections, and a surprise

[Kyle Moffett]

On Jul 01, 2004, at 00:11, Jamie Lokier wrote:
> Can you confirm in a simple way that mapping a file, or some anonymous
> memory, without PROT_READ, really isn't writable under MacOS X?  Can
> you confirm it with a word write, if that would be relevant?

I hope I didn't make some stupid mistake in my program, but here it is, 
and
here are my results.  I'll probably go file a bug with Apple now :-D

zeus:~ kylemoffett$ cat >testp.c
#include <sys/types.h>
#include <sys/mman.h>

#include <stdlib.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/mman.h>

int main(int argc, char **argv) {
         long x = 100;
         void *mem;
         fprintf(stderr,"Starting...\n");
         mem = mmap(0,4096,PROT_WRITE,MAP_ANON|MAP_SHARED,-1,0);
         fprintf(stderr,"Mapped memory!\n");
         if (mem == 0) return 1;
         fprintf(stderr,"Address is %lx\n",(unsigned long)mem);
         ((long *)mem)[1] = x;
         fprintf(stderr,"Done!!!\n");
         return 0;
}
^D
zeus:~ kylemoffett$ gcc testp.c -o testp
zeus:~ kylemoffett$ ./testp
Starting...
Mapped memory!
Address is 4000
Bus error
zeus:~ kylemoffett$

Cheers,
Kyle Moffett

Re: Testing PROT_NONE and other protections, and a surprise

[Jamie Lokier]

Kyle Moffett wrote:
> >Can you confirm in a simple way that mapping a file, or some anonymous
> >memory, without PROT_READ, really isn't writable under MacOS X?  Can
> >you confirm it with a word write, if that would be relevant?
> 
> I hope I didn't make some stupid mistake in my program, but here it
> is, and here are my results.

Thanks for testing, Kyle.

It looks fine, although this is wrong:

>         mem = mmap(0,4096,PROT_WRITE,MAP_ANON|MAP_SHARED,-1,0);
> ...
>         if (mem == 0) return 1;

The error code is -1, aka. MAP_FAILED.

> Starting...
> Mapped memory!
> Address is 4000

That's a surprisingly low address.

> Bus error

Phew, I'm glad I decided to catch SIGBUS in the test program at the
last moment...

That's a historical BSD-ism.  They can't change it now, because
programs do trap and check for SIGBUS on that platform for protection
violations.

> I'll probably go file a bug with Apple now :-D

It might be a generic *BSD bug (for whatever value of * is used by MacOS X).

That would be interesting to know -- anyone here running *BSD on PPC
or any other architecture to test?

Of course it's an Apple bug as well :)

-- Jamie

Re: [OT] Testing PROT_NONE and other protections, and a surprise

[Kyle Moffett]

On Jul 01, 2004, at 08:39, Jamie Lokier wrote:
> The error code is -1, aka. MAP_FAILED.
Oops!  I guess I was just lucky that part didn't fail :-D  On the other 
hand, it
couldn't legally return 0 anyway, could it?  That would have been a 
slightly
more sensible error code, IMHO, anyway, but it probably came from some
silly standard somewhere.

>> I'll probably go file a bug with Apple now :-D
>
> It might be a generic *BSD bug (for whatever value of * is used by 
> MacOS X).
>
> That would be interesting to know -- anyone here running *BSD on PPC
> or any other architecture to test?
>
> Of course it's an Apple bug as well :)

Apple's BSD derivative came out of the main tree several years ago, and 
it
wasn't really maintained for a few years, so it missed out on a lot of 
bug
fixes and such.  They've tried to catch up on a lot of that and been 
mildly
successful, but it still has a ways to go.

Cheers,
Kyle Moffett

Re: [OT] Testing PROT_NONE and other protections, and a surprise

[Jamie Lokier]

Kyle Moffett wrote:
> >The error code is -1, aka. MAP_FAILED.
> Oops!  I guess I was just lucky that part didn't fail :-D On the
> other hand, it couldn't legally return 0 anyway, could it?

Yes it could -- if you request a mapping at address 0 with MAP_FIXED.
A few OSes won't do that, but Linux and many others will.

-- Jamie

Re: [OT] Testing PROT_NONE and other protections, and a surprise

[Kyle Moffett]

On Jul 01, 2004, at 10:50, Jamie Lokier wrote:
> Kyle Moffett wrote:
>>> The error code is -1, aka. MAP_FAILED.
>> Oops!  I guess I was just lucky that part didn't fail :-D On the
>> other hand, it couldn't legally return 0 anyway, could it?
>
> Yes it could -- if you request a mapping at address 0 with MAP_FIXED.
> A few OSes won't do that, but Linux and many others will.

That allows untrapped dereferencing of a NULL pointer.  IMHO, that
would be a very unintelligent thing for a program to do, to deny itself
the bug-catching features provided therein, but it's interesting to see
that it is possible.

#include <sys/types.h>
#include <sys/mman.h>

int main() {
	void *mem = mmap(0,4096,PROT_WRITE,MAP_FIXED|MAP_ANON|MAP_SHARED,-1,0);
	if (mem == MAP_FAILED) return 1;
	((long *)mem)[0] = 1;
	return 0;
}

Cheers,
Kyle Moffett

Re: [OT] Testing PROT_NONE and other protections, and a surprise

[Matt Mackall]

On Thu, Jul 01, 2004 at 11:01:52AM -0400, Kyle Moffett wrote:
> On Jul 01, 2004, at 10:50, Jamie Lokier wrote:
> >Kyle Moffett wrote:
> >>>The error code is -1, aka. MAP_FAILED.
> >>Oops!  I guess I was just lucky that part didn't fail :-D On the
> >>other hand, it couldn't legally return 0 anyway, could it?
> >
> >Yes it could -- if you request a mapping at address 0 with MAP_FIXED.
> >A few OSes won't do that, but Linux and many others will.
> 
> That allows untrapped dereferencing of a NULL pointer.  IMHO, that
> would be a very unintelligent thing for a program to do, to deny itself
> the bug-catching features provided therein, but it's interesting to see
> that it is possible.

A typical use is vm86-based emulation of 16-bit DOS where there's data
in the immediate vicinity of NULL.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [OT] Testing PROT_NONE and other protections, and a surprise

[Michael Driscoll]

On Thursday 01 July 2004 08:43, Kyle Moffett wrote:
> On Jul 01, 2004, at 08:39, Jamie Lokier wrote:
> > The error code is -1, aka. MAP_FAILED.
>
> Oops!  I guess I was just lucky that part didn't fail :-D  On the other
> hand, it couldn't legally return 0 anyway, could it?  That would have been a
> slightly more sensible error code, IMHO, anyway, but it probably came from
> some silly standard somewhere.

mmap can (and will, in the case of glibc) return NULL for a length of 0.

-- 
Michael Driscoll, fenris@ulfheim.net
"A noble spirit embiggens the smallest man" -- J. Springfield

Re: [OT] Testing PROT_NONE and other protections, and a surprise

[Gabriel Paubert]

On Thu, Jul 01, 2004 at 10:43:05AM -0400, Kyle Moffett wrote:
> On Jul 01, 2004, at 08:39, Jamie Lokier wrote:
> >The error code is -1, aka. MAP_FAILED.
> Oops!  I guess I was just lucky that part didn't fail :-D  On the other 
> hand, it
> couldn't legally return 0 anyway, could it?  That would have been a 
> slightly
> more sensible error code, IMHO, anyway, but it probably came from some
> silly standard somewhere.

It can, but only if you explicitly ask for it (MAP_FIXED with 0 as an
address), and it is very useful for DOS emulators.
	
	Cheers,
	Gabriel

Re: Testing PROT_NONE and other protections, and a surprise

[Russell King]

On Thu, Jul 01, 2004 at 04:26:06AM +0100, Jamie Lokier wrote:
> When running it on i386, I got a *huge* surprise (to me).  A
> PROT_WRITE-only page can sometimes fault on read or exec.  This is the
> output:
> 
> Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX
> ========================================================================
> MAP_SHARED     | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx
> MAP_PRIVATE    | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx
> 
> The "!" means that a read or exec *sometimes* raises a signal.

Here are the ARM results:

Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX
========================================================================
MAP_SHARED     | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx
MAP_PRIVATE    | ---    r-x    !w!    rwx    r-x    r-x    rwx    rwx

As expected, the same as x86 since we have the same situation there -
we can not represent the write-only page permission in hardware.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: Testing PROT_NONE and other protections, and a surprise

[Richard Curnow]

Hi Jamie,

* Jamie Lokier <jamie@shareable.org> [2004-07-01]:
> 
> I've just written a thorough test.  The attached program tries every
> combination of PROT_* flags, and tells you what protection you really get.
> 
> It'll be interesting to see the results on other architectures.

I've got this working on sh64/2.6 (which was only merged a couple of
days ago); here are the results:

Requested PROT | ---    R--    -W-    RW-    --X    R-X    -WX    RWX
========================================================================
MAP_SHARED     | ---    r--    -w-    rw-    r-x    r-x    rwx    rwx
MAP_PRIVATE    | ---    r--    -w-    rw-    r-x    r-x    rwx    rwx

Although the hardware is capable of distinguish R and X, the kernel
always allows R if X is specified to mmap().  This is for 2 reasons :

1. jump tables for switch() get embedded into the code in 32-bit
   (SHmedia) mode
2. constant pools embedded in the code in 16-bit (SHcompact, i.e. SH-4
   compatible) mode

so in practice an exec-only page is pretty useless to a typical userland
program.

> This program should hopefully run on all architectures, however it
> does depend on an empty function working when relocated.

The empty function relocated fine.  But I had to make 2 trivial changes to
handle using &void_function as an argument to memcpy and when casting
addr to a function pointer.  These result from the way the SH-5 uses the
LSB in function addresses and branch targets to switch between the
SHmedia and SHcompact instruction sets.  (I can send you the patch if you
want.)

-- 
Richard \\\ SH-4/SH-5 Core & Debug Architect
Curnow  \\\         SuperH (UK) Ltd, Bristol

Re: A question about PROT_NONE on ARM and ARM26

[Russell King]

On Wed, Jun 30, 2004 at 03:44:34AM +0100, Jamie Lokier wrote:
> My question is: if the _kernel_ reads a PROT_NONE page, will it fault?
> It looks likely to me.

There are two different types of privileged accesses on ARM.  One is the
standard load/store instruction, which checks the permissions for the
current processor mode.  The other is one which simulates a user mode
access to the address.

We use the latter for get_user/put_user/copy_to_user/copy_from_user.

> This means that calling write() with a PROT_NONE region would succeed,
> wouldn't it?

No, because the uaccess.h function will fault, and we'll end up returning
-EFAULT.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: A question about PROT_NONE on ARM and ARM26

[Jamie Lokier]

Russell King wrote:
> There are two different types of privileged accesses on ARM.  One is the
> standard load/store instruction, which checks the permissions for the
> current processor mode.  The other is one which simulates a user mode
> access to the address.
> 
> We use the latter for get_user/put_user/copy_to_user/copy_from_user.
> 
> > This means that calling write() with a PROT_NONE region would succeed,
> > wouldn't it?
> 
> No, because the uaccess.h function will fault, and we'll end up returning
> -EFAULT.

Ok, that answers my question, thanks.  ARM and ARM26 are fine with PROT_NONE.

Those are the "ldrlst" instructions in getuser.S, right?

Here's a question, for ARM only (not ARM26):
...........................................

getuser.S uses "ldrlst", but unlike ARM26 has no TASK_SIZE check and
matching "ldrge".  If kernel C code uses set_fs(), then get_user()
_should_ permit reading from kernel addresses.  Will that work on ARM?

I ask because it's interesting to see that ARM and ARM26 have quite
different code in getuser.S and putuser.S.  The ARM code is shorter.

Here's an optimisation idea, for ARM26 only:
...........................................

Do you need the "strlst" instructions in putuser.S?  They're followed
by "strge" instructions.

For storing, it looks as though the protections set in pgtable.h will
trigger a write fault whether it's a user mode access or not.  Thus
you _might_ be able to shave an instruction or two off each put_user,
by simply doing a single unconditional kernel mode store.  (The check
against TASK_SIZE has already been done).

Just an idea, I don't know ARM26 well enough to know if that'd work.

-- Jamie

Re: A question about PROT_NONE on ARM and ARM26

[Ian Molton]

On Wed, 30 Jun 2004 15:59:42 +0100
Jamie Lokier <jamie@shareable.org> wrote:

> 
> Here's an optimisation idea, for ARM26 only:
> ...........................................
> 
> Do you need the "strlst" instructions in putuser.S?  They're followed
> by "strge" instructions.

ARM26 is special compared to some other architectures.

the CPU has a 64MB address space, and in all known ARM26 + MMU
configurations, the bottom 32MB are the logical addresses. the upper
32MB (where kernel, physical RAM (16MB max) and IO live) are physically
addressable ONLY.

the kernel isnt mapped into the virtual address space on ARM26. it could
be, but with only 512 logical pages maximum on a normal machine (1024 on
a machine with very little RAM) it would cripple the system even more
than it already is.

the tests in ARM26 determine wether to use a translated access or a
nontranslated one depending on wether we access kernel or user space.

Re: A question about PROT_NONE on ARM and ARM26

[Russell King]

On Wed, Jun 30, 2004 at 03:59:42PM +0100, Jamie Lokier wrote:
> Russell King wrote:
> > There are two different types of privileged accesses on ARM.  One is the
> > standard load/store instruction, which checks the permissions for the
> > current processor mode.  The other is one which simulates a user mode
> > access to the address.
> > 
> > We use the latter for get_user/put_user/copy_to_user/copy_from_user.
> > 
> > > This means that calling write() with a PROT_NONE region would succeed,
> > > wouldn't it?
> > 
> > No, because the uaccess.h function will fault, and we'll end up returning
> > -EFAULT.
> 
> Ok, that answers my question, thanks.  ARM and ARM26 are fine with PROT_NONE.
> 
> Those are the "ldrlst" instructions in getuser.S, right?
> 
> Here's a question, for ARM only (not ARM26):
> ...........................................
> 
> getuser.S uses "ldrlst", but unlike ARM26 has no TASK_SIZE check and
> matching "ldrge".  If kernel C code uses set_fs(), then get_user()
> _should_ permit reading from kernel addresses.  Will that work on ARM?

Indeed it does - it's all magic.  Firstly, let me explain "ldrlst".  This
is "ldr" + "ls" + "t".  "ldr" = load register.  "ls" = less than (all
instructions are conditional on ARM.)  "t" = the magic which turns this
access into a user mode access.

If the address is larger than the value in TI_ADDR_LIMIT, there's no
point in even trying the access - it will fail, so we just do the "bad
access" handling.  This also happens if the instruction faults and the
fault can not be fixed up.

However, when we have set_fs(KERNEL_DS) in effect, we modify two things.
First is the TI_ADDR_LIMIT, which allows any access through the assembly
check.  The other is the magic - we fiddle with the domain register.

Every translation has a "domain" index associated with it, and each
domain can be in one of three modes: no access, client or manager.

If it's in "no access" mode, nothing can access translations in this
domain.  "client" mode means that the page level permissions are checked
and faults are generated depending on the access mode vs the permission
mode.  "manager" means the page level permissions are not checked at
all, and any access will succeed irrespective of the page level
permissions.

We use three domains - one for user, one for kernel and one for IO.
Normally all three are in client mode.  However, on set_fs(KERNEL_DS)
we switch the kernel domain to manager mode.

This means that the user-mode LDR instructions (ldrt / ldrlst etc)
will not have their page permissions checked, and therefore the access
will succeed - exactly as we require.

> I ask because it's interesting to see that ARM and ARM26 have quite
> different code in getuser.S and putuser.S.  The ARM code is shorter.

ARM26 is completely different - it doesn't have the ability to bypass
permission checks in the "kernel" area of memory.  Therefore, ARM26
has to rely solely on the TI_ADDR_LIMIT check and select the appropriate
instruction to use based upon the suceeding address.

> Here's an optimisation idea, for ARM26 only:
> ...........................................
> 
> Do you need the "strlst" instructions in putuser.S?  They're followed
> by "strge" instructions.

The outcome of the page permission checks are slightly different for the
strt vs str instructions for both the ARM26 cases:

Privileged  T-bit     00      01     10         11
    Y         0       r/w     r/w    r/w        r/w
    Y         1       r/w     read   no access  no access
    N         X       r/w     read   no access  no access

Note: if PAGE_NOT_USER and PAGE_OLD are both clear (iow, young + user
page) we use bit pattern 0x.  If PAGE_NOT_USER, PAGE_OLD, PAGE_READONLY
and PAGE_CLEAN are all clear, we use bit pattern 00.  Otherwise we use
bit pattern 11.

We have a similar difference in kernel-mode vs user-mode accesses for
the ARM case as well - so its all complicated and unless you really
understand this... 8)

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: A question about PROT_NONE on ARM and ARM26

[Jamie Lokier]

Russell King wrote:
> We use three domains - one for user, one for kernel and one for IO.
> Normally all three are in client mode.  However, on set_fs(KERNEL_DS)
> we switch the kernel domain to manager mode.
> 
> This means that the user-mode LDR instructions (ldrt / ldrlst etc)
> will not have their page permissions checked, and therefore the access
> will succeed - exactly as we require.

Protection permissions (i.e. read-only, PROT_NONE) should still be
checked after set_fs(KERNEL_DS).  It's only the kernel page vs. user
page distinction that should be relaxed.

>From your description, it's not obvious that it'll do the right thing
in that circumstance.

Hopefully,

> [Tables]
> We have a similar difference in kernel-mode vs user-mode accesses for
> the ARM case as well - so its all complicated and unless you really
> understand this... 8)

...this is alluding to a mechanism such that exactly the right thing
happens for PROT_NONE and PROT_READONLY pages after set_fs(KERNEL_DS), yes?

> Privileged  T-bit     00      01     10         11
>     Y         0       r/w     r/w    r/w        r/w
>     Y         1       r/w     read   no access  no access
>     N         X       r/w     read   no access  no access
> 
> Note: if PAGE_NOT_USER and PAGE_OLD are both clear (iow, young + user
> page) we use bit pattern 0x.  If PAGE_NOT_USER, PAGE_OLD, PAGE_READONLY
> and PAGE_CLEAN are all clear, we use bit pattern 00.  Otherwise we use
> bit pattern 11.

Ok, that explains nicely and should do the right thing on ARM26 with
PROT_NONE pages, even with set_fs(KERNEL_DS).

Because set_fs() is rarely used, I think you can optimise getuser.S
and putuser.S on ARM26.  Instead of comparing the address against
TI_ADDR_LIMIT, compare it against the hard-coded userspace limit.

If that succeeds, continue with ldrt et al.  Note the improvements in
the common case (fs == USER_DS and no fault): (1) you only compare
against one limit, not two; (2) no load of TI_ADDR_LIMIT; (3) one less
ldr instruction.

If that comparison fails, then branch to a version which checks
TI_ADDR_LIMIT.

Here's an example.  It's probably wrong as I haven't written ARM in a
long time, but illustrates the idea.  Note how the common case takes 4
instructions instead of 12 in the current code:

__get_user_4:
	cmp	r0,#0x02000000
4:	ldrlst	r1, [r0]
	movls	r0, #0
	movls	pc, lr
	bic	r1, sp, #0x1f00
	bic	r1, r1, #0x00ff
	str	lr, [sp, #-4]!
	ldr	r1, [r1, #TI_ADDR_LIMIT]
	sub	r1, r1, #4
	cmp	r0, r1
14:	ldrls	r1, [r0]
	movls	r0, #0
	ldmfdls	sp!, {pc}^
	b	__get_user_bad

-- Jamie

Re: A question about PROT_NONE on ARM and ARM26

[Russell King]

On Wed, Jun 30, 2004 at 08:14:28PM +0100, Jamie Lokier wrote:
> Russell King wrote:
> > We use three domains - one for user, one for kernel and one for IO.
> > Normally all three are in client mode.  However, on set_fs(KERNEL_DS)
> > we switch the kernel domain to manager mode.
> > 
> > This means that the user-mode LDR instructions (ldrt / ldrlst etc)
> > will not have their page permissions checked, and therefore the access
> > will succeed - exactly as we require.
> 
> Protection permissions (i.e. read-only, PROT_NONE) should still be
> checked after set_fs(KERNEL_DS).  It's only the kernel page vs. user
> page distinction that should be relaxed.
> 
> >From your description, it's not obvious that it'll do the right thing
> in that circumstance.

Trust me, it does.  Unless you fully understand how the MMU and domains
work on ARM, you've little chance of working it out from the code.

Really, I see its pointless trying to discuss the details of this any
further - I presently have very little time to educate people in the
details, sorry.

> Because set_fs() is rarely used, I think you can optimise getuser.S
> and putuser.S on ARM26.  Instead of comparing the address against
> TI_ADDR_LIMIT, compare it against the hard-coded userspace limit.

Wrong.  That means that if userspace passes an address above the hard
coded limit, we _WILL_ bypass all protections and access that memory.

However, ARM26 is not under my control anymore, so it isn't something
I care about, and I doubt there are many people who do.  We're talking
about a 20 year old architecture which hasn't had any conforming devices
produced for at least 10 years.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: A question about PROT_NONE on ARM and ARM26

[Jamie Lokier]

Russell King wrote:
> Trust me, it does.  Unless you fully understand how the MMU and domains
> work on ARM, you've little chance of working it out from the code.

Thanks, that's fine.  I just wanted you to confirm PROT_NONE works
with set_fs(KERNEL_DS), as it's not apparent from your earlier
description.  I don't need to know _how_ it works - I can read manuals
too - although you description was interesting.

> > Instead of comparing the address against TI_ADDR_LIMIT, compare it
> > against the hard-coded userspace limit.
> 
> Wrong.  That means that if userspace passes an address above the hard
> coded limit, we _WILL_ bypass all protections and access that memory.

No - it does check against TI_ADDR_LIMIT in the case that the address
is above the hard-coded limit, so prevents that.

The optimisation is valid on all architectures, actually, including
current ARM where it saves a few instructions in the common path.

Here's the potential improvement to current 32-bit ARM.  It's
4 instructions instead of 8 and one less load, in the common case:

__get_user_4:
	cmp	r0, #TASK_SIZE-4
4:	ldrlet	r1, [r0]
	movle	r0, #0
	movle	pc, lr
	bic	r1, sp, #0x1f00
	bic	r1, r1, #0x00ff
	ldr	r1, [r1, #TI_ADDR_LIMIT]
	sub	r1, r1, #4
	cmp	r0, r1
14:	ldrlet	r1, [r0]
	movle	r0, #0
	movle	pc, lr
	b	__get_user_bad

Finally, I think I see a bug in current ARM.  Shouldn't this use
ldrlet instead of ldrlst?  Think about accesses to addresses
TASK_SIZE-4 and 0xfffffffc.

	ldr	r1, [r1, #TI_ADDR_LIMIT]
	sub	r1, r1, #4
	cmp	r0, r1
4:	ldrlst	r1, [r0]

Thanks,
-- Jamie

Re: A question about PROT_NONE on ARM and ARM26

[Russell King]

On Wed, Jun 30, 2004 at 09:15:46PM +0100, Jamie Lokier wrote:
> Russell King wrote:
> > Trust me, it does.  Unless you fully understand how the MMU and domains
> > work on ARM, you've little chance of working it out from the code.
> 
> Thanks, that's fine.  I just wanted you to confirm PROT_NONE works
> with set_fs(KERNEL_DS), as it's not apparent from your earlier
> description.  I don't need to know _how_ it works - I can read manuals
> too - although you description was interesting.

Ok, to fill in for just this bit, the domain covering user space mappings
always remains in "client" mode, so page protections are always checked.
PAGE_NONE does not have the "user" bit set, so both user space accesses
and ldrt/strt instructions will be unable to access the pages, which is
the desired behaviour.

However, plain ldr and str instructions will access the page, but
get_user/put_user doesn't use them, and copy_from_user/copy_to_user
are carefully crafted to ensure that we hit the necessary permission
checks for each page it touches on the first access.

> > > Instead of comparing the address against TI_ADDR_LIMIT, compare it
> > > against the hard-coded userspace limit.
> > 
> > Wrong.  That means that if userspace passes an address above the hard
> > coded limit, we _WILL_ bypass all protections and access that memory.
> 
> No - it does check against TI_ADDR_LIMIT in the case that the address
> is above the hard-coded limit, so prevents that.

Ok.

> Here's the potential improvement to current 32-bit ARM.  It's
> 4 instructions instead of 8 and one less load, in the common case:
> 
> __get_user_4:
> 	cmp	r0, #TASK_SIZE-4
> 4:	ldrlet	r1, [r0]
> 	movle	r0, #0
> 	movle	pc, lr
> 	bic	r1, sp, #0x1f00
> 	bic	r1, r1, #0x00ff
> 	ldr	r1, [r1, #TI_ADDR_LIMIT]
> 	sub	r1, r1, #4
> 	cmp	r0, r1
> 14:	ldrlet	r1, [r0]
> 	movle	r0, #0
> 	movle	pc, lr
> 	b	__get_user_bad

Ok, this could work, but there's one gotcha - TASK_SIZE-4 doesn't fit
in an 8-bit rotated constants, so we need 2 extra instructions:

__get_user_4:
	mov	r1, #TASK_SIZE
	sub	r1, r1, #4
	cmp	r0, r1
4:	ldrlet	r1, [r0]
	movle	r0, #0
	movle	pc, lr
	...

> Finally, I think I see a bug in current ARM.  Shouldn't this use
> ldrlet instead of ldrlst?  Think about accesses to addresses
> TASK_SIZE-4 and 0xfffffffc.

LS = unsigned less than or same.  LE = signed less than or equal.  You
need the unsigned compare because addresses are unsigned.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: A question about PROT_NONE on ARM and ARM26

[Jamie Lokier]

Russell King wrote:
> > Here's the potential improvement to current 32-bit ARM.  It's
> > 4 instructions instead of 8 and one less load, in the common case:
> > 
> > __get_user_4:
> > 	cmp	r0, #TASK_SIZE-4
> > 4:	ldrlet	r1, [r0]
> > 	movle	r0, #0
> > 	movle	pc, lr
> > 	bic	r1, sp, #0x1f00
> > 	bic	r1, r1, #0x00ff
> > 	ldr	r1, [r1, #TI_ADDR_LIMIT]
> > 	sub	r1, r1, #4
> > 	cmp	r0, r1
> > 14:	ldrlet	r1, [r0]
> > 	movle	r0, #0
> > 	movle	pc, lr
> > 	b	__get_user_bad
> 
> Ok, this could work, but there's one gotcha - TASK_SIZE-4 doesn't fit
> in an 8-bit rotated constants, so we need 2 extra instructions:
> 
> __get_user_4:
> 	mov	r1, #TASK_SIZE
> 	sub	r1, r1, #4
> 	cmp	r0, r1
> 4:	ldrlet	r1, [r0]
> 	movle	r0, #0
> 	movle	pc, lr
> 	...

One more possibility:

	cmp	r0, #(TASK_SIZE - (1<<24))

I.e. just compare against the largest constant that can be
represented.  For accesses to the last part of userspace, it's a
penalty of 4 instructions -- but it might work out to be a net gain.

Actually, since the shortest path is only three instructions in the
fast case, not counting control flow, it might be good to inline those
3 in uaccess.h, and change the "bl" to a conditonal "blhi" there.

> > Finally, I think I see a bug in current ARM.  Shouldn't this use
> > ldrlet instead of ldrlst?  Think about accesses to addresses
> > TASK_SIZE-4 and 0xfffffffc.
> 
> LS = unsigned less than or same.  LE = signed less than or equal.  You
> need the unsigned compare because addresses are unsigned.

Ah.  I was guessing the mnemonic.

That's because of the way "ge" is used on ARM26 in places, which
therefore look buggy or subtly clever:

        ldr     r1, [r1, #TI_ADDR_LIMIT]
        sub     r1, r1, #4
        cmp     r0, r1
        bge     __get_user_bad
        cmp     r0, #0x02000000
4:      ldrlst  r1, [r0]
        ldrge   r1, [r0]

"ge" is a signed comparison, and unsigned is needed here, unless I
missed something subtle.  So "bge" and "ldrge" should be "bhi" and "ldrhi".

Thanks,
-- Jamie

Re: A question about PROT_NONE on ARM and ARM26

[Ian Molton]

On Thu, 1 Jul 2004 00:30:14 +0100
Jamie Lokier <jamie@shareable.org> wrote:

> "ge" is a signed comparison, and unsigned is needed here, unless I
> missed something subtle.  So "bge" and "ldrge" should be "bhi" and "ldrhi".

technically, I think you're right here.

in practise, the arm26 address space is too small (64MB) for this to
ever cause a problem.

Re: A question about PROT_NONE on ARM and ARM26

[Jamie Lokier]

Ian Molton wrote:
> > "ge" is a signed comparison, and unsigned is needed here, unless I
> > missed something subtle.  So "bge" and "ldrge" should be "bhi" and "ldrhi".
> 
> technically, I think you're right here.
> 
> in practise, the arm26 address space is too small (64MB) for this to
> ever cause a problem.

No -- there is still a bug.

The bug is that userspace can pass an address like 0x90000000 to the
kernel.  This is possible even on arm26.

If you follow the logic in getuser.S, it won't branch to
__get_user_bad, and it won't execute _either_ of the "ldrlst" or
"ldrge" instructions.

So it'll end up returning the value that happens to be in r1 and/or
r2, and using that for the syscall, instead of the syscall returning
-EFAULT as it should.

In rare cases, that's a security information leakage.  Usually it's
just rubbish.

-- Jamie

Re: A question about PROT_NONE on ARM and ARM26

[Nicolas Pitre]

On Thu, 1 Jul 2004, Jamie Lokier wrote:

> Russell King wrote:
> > Ok, this could work, but there's one gotcha - TASK_SIZE-4 doesn't fit
> > in an 8-bit rotated constants, so we need 2 extra instructions:
> > 
> > __get_user_4:
> > 	mov	r1, #TASK_SIZE
> > 	sub	r1, r1, #4
> > 	cmp	r0, r1
> > 4:	ldrlet	r1, [r0]
> > 	movle	r0, #0
> > 	movle	pc, lr
> > 	...
> 
> One more possibility:
> 
> 	cmp	r0, #(TASK_SIZE - (1<<24))
> 
> I.e. just compare against the largest constant that can be
> represented.  For accesses to the last part of userspace, it's a
> penalty of 4 instructions -- but it might work out to be a net gain.

Maybe not.  The user stack is located at the top so any user buffer 
allocated on the stack would be penalized.


Nicolas

Re: A question about PROT_NONE on ARM and ARM26

[Jamie Lokier]

Nicolas Pitre wrote:
> > 	cmp	r0, #(TASK_SIZE - (1<<24))
> > 
> > I.e. just compare against the largest constant that can be
> > represented.  For accesses to the last part of userspace, it's a
> > penalty of 4 instructions -- but it might work out to be a net gain.
> 
> Maybe not.  The user stack is located at the top so any user buffer 
> allocated on the stack would be penalized.

I agree.  I don't know if it would work out to be a net gain on
average or a net loss.

It saves a couple of instructions, but when it fails the cost is only
a few instructions anyway.

Probably for get_user & put_user, the common case _is_ to be on the
user's stack, so Russell's code would be better.

-- Jamie

Re: A question about PROT_NONE on ARM and ARM26

[Russell King]

On Thu, Jul 01, 2004 at 12:30:14AM +0100, Jamie Lokier wrote:
> > Ok, this could work, but there's one gotcha - TASK_SIZE-4 doesn't fit
> > in an 8-bit rotated constants, so we need 2 extra instructions:
> > 
> > __get_user_4:
> > 	mov	r1, #TASK_SIZE
> > 	sub	r1, r1, #4
> > 	cmp	r0, r1
> > 4:	ldrlet	r1, [r0]
> > 	movle	r0, #0
> > 	movle	pc, lr
> > 	...
> 
> One more possibility:
> 
> 	cmp	r0, #(TASK_SIZE - (1<<24))
> 
> I.e. just compare against the largest constant that can be
> represented.  For accesses to the last part of userspace, it's a
> penalty of 4 instructions -- but it might work out to be a net gain.
> 
> Actually, since the shortest path is only three instructions in the
> fast case, not counting control flow, it might be good to inline those
> 3 in uaccess.h, and change the "bl" to a conditonal "blhi" there.

I've been wondering whether we need any of this checking for 32-bit
ARMs in any case.  I wouldn't like to say definitely, but I've just
tried this:

__get_user_4:
4:	ldrt	r1, [r0]
	mov	r0, #0
	mov	pc, lr

and it appears to behave as we want.  My only concern is Xscale with
some of their errata.

If this works out fine, then we could kill the differences between
__get_user and get_user, and __put_user and put_user on ARM.  Before
doing that, however, I would want to evaluate the performance and
size difference between having this out of line and inline.

diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' orig/arch/arm/lib/getuser.S linux/arch/arm/lib/getuser.S
--- orig/arch/arm/lib/getuser.S	Tue Apr 13 19:40:00 2004
+++ linux/arch/arm/lib/getuser.S	Fri Jul  2 14:26:32 2004
@@ -32,59 +32,34 @@
 
 	.global	__get_user_1
 __get_user_1:
-	bic	r1, sp, #0x1f00
-	bic	r1, r1, #0x00ff
-	ldr	r1, [r1, #TI_ADDR_LIMIT]
-	sub	r1, r1, #1
-	cmp	r0, r1
-1:	ldrlsbt	r1, [r0]
-	movls	r0, #0
-	movls	pc, lr
-	b	__get_user_bad
+1:	ldrbt	r1, [r0]
+	mov	r0, #0
+	mov	pc, lr
 
 	.global	__get_user_2
 __get_user_2:
-	bic	r2, sp, #0x1f00
-	bic	r2, r2, #0x00ff
-	ldr	r2, [r2, #TI_ADDR_LIMIT]
-	sub	r2, r2, #2
-	cmp	r0, r2
-2:	ldrlsbt	r1, [r0], #1
-3:	ldrlsbt	r2, [r0]
+2:	ldrbt	r1, [r0], #1
+3:	ldrbt	r2, [r0]
 #ifndef __ARMEB__
-	orrls	r1, r1, r2, lsl #8
+	orr	r1, r1, r2, lsl #8
 #else
-	orrls	r1, r2, r1, lsl #8
+	orr	r1, r2, r1, lsl #8
 #endif
-	movls	r0, #0
-	movls	pc, lr
-	b	__get_user_bad
+	mov	r0, #0
+	mov	pc, lr
 
 	.global	__get_user_4
 __get_user_4:
-	bic	r1, sp, #0x1f00
-	bic	r1, r1, #0x00ff
-	ldr	r1, [r1, #TI_ADDR_LIMIT]
-	sub	r1, r1, #4
-	cmp	r0, r1
-4:	ldrlst	r1, [r0]
-	movls	r0, #0
-	movls	pc, lr
-	b	__get_user_bad
+4:	ldrt	r1, [r0]
+	mov	r0, #0
+	mov	pc, lr
 
 	.global	__get_user_8
 __get_user_8:
-	bic	r2, sp, #0x1f00
-	bic	r2, r2, #0x00ff
-	ldr	r2, [r2, #TI_ADDR_LIMIT]
-	sub	r2, r2, #8
-	cmp	r0, r2
-5:	ldrlst	r1, [r0], #4
-6:	ldrlst	r2, [r0]
-	movls	r0, #0
-	movls	pc, lr
-
-	/* fall through */
+5:	ldrt	r1, [r0], #4
+6:	ldrt	r2, [r0]
+	mov	r0, #0
+	mov	pc, lr
 
 __get_user_bad_8:
 	mov	r2, #0
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' orig/arch/arm/lib/putuser.S linux/arch/arm/lib/putuser.S
--- orig/arch/arm/lib/putuser.S	Tue Apr 13 19:40:00 2004
+++ linux/arch/arm/lib/putuser.S	Fri Jul  2 14:27:14 2004
@@ -32,60 +32,35 @@
 
 	.global	__put_user_1
 __put_user_1:
-	bic	ip, sp, #0x1f00
-	bic	ip, ip, #0x00ff
-	ldr	ip, [ip, #TI_ADDR_LIMIT]
-	sub	ip, ip, #1
-	cmp	r0, ip
-1:	strlsbt	r1, [r0]
-	movls	r0, #0
-	movls	pc, lr
-	b	__put_user_bad
+1:	strbt	r1, [r0]
+	mov	r0, #0
+	mov	pc, lr
 
 	.global	__put_user_2
 __put_user_2:
-	bic	ip, sp, #0x1f00
-	bic	ip, ip, #0x00ff
-	ldr	ip, [ip, #TI_ADDR_LIMIT]
-	sub	ip, ip, #2
-	cmp	r0, ip
 	movls	ip, r1, lsr #8
 #ifndef __ARMEB__
-2:	strlsbt	r1, [r0], #1
-3:	strlsbt	ip, [r0]
+2:	strbt	r1, [r0], #1
+3:	strbt	ip, [r0]
 #else
-2:	strlsbt	ip, [r0], #1
-3:	strlsbt	r1, [r0]
+2:	strbt	ip, [r0], #1
+3:	strbt	r1, [r0]
 #endif
-	movls	r0, #0
-	movls	pc, lr
-	b	__put_user_bad
+	mov	r0, #0
+	mov	pc, lr
 
 	.global	__put_user_4
 __put_user_4:
-	bic	ip, sp, #0x1f00
-	bic	ip, ip, #0x00ff
-	ldr	ip, [ip, #TI_ADDR_LIMIT]
-	sub	ip, ip, #4
-	cmp	r0, ip
-4:	strlst	r1, [r0]
-	movls	r0, #0
-	movls	pc, lr
-	b	__put_user_bad
+4:	strt	r1, [r0]
+	mov	r0, #0
+	mov	pc, lr
 
 	.global	__put_user_8
 __put_user_8:
-	bic	ip, sp, #0x1f00
-	bic	ip, ip, #0x00ff
-	ldr	ip, [ip, #TI_ADDR_LIMIT]
-	sub	ip, ip, #8
-	cmp	r0, ip
-5:	strlst	r1, [r0], #4
-6:	strlst	r2, [r0]
-	movls	r0, #0
-	movls	pc, lr
-
-	/* fall through */
+5:	strt	r1, [r0], #4
+6:	strt	r2, [r0]
+	mov	r0, #0
+	mov	pc, lr
 
 __put_user_bad:
 	mov	r0, #-EFAULT

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: A question about PROT_NONE on ARM and ARM26

[Scott Wood]

On Wed, Jun 30, 2004 at 11:59:22PM +0100, Russell King wrote:
> Ok, to fill in for just this bit, the domain covering user space mappings
> always remains in "client" mode, so page protections are always checked.
> PAGE_NONE does not have the "user" bit set, so both user space accesses
> and ldrt/strt instructions will be unable to access the pages, which is
> the desired behaviour.
> 
> However, plain ldr and str instructions will access the page, but
> get_user/put_user doesn't use them, and copy_from_user/copy_to_user
> are carefully crafted to ensure that we hit the necessary permission
> checks for each page it touches on the first access.

What if CONFIG_PREEMPT is enabled, and you get preempted after that
first access, and another thread unmaps the page before you're
finished with it?

-Scott

Re: A question about PROT_NONE on ARM and ARM26

[Jamie Lokier]

Scott Wood wrote:
> > However, plain ldr and str instructions will access the page, but
> > get_user/put_user doesn't use them, and copy_from_user/copy_to_user
> > are carefully crafted to ensure that we hit the necessary permission
> > checks for each page it touches on the first access.
> 
> What if CONFIG_PREEMPT is enabled, and you get preempted after that
> first access, and another thread unmaps the page before you're
> finished with it?

The code in uaccess.S:__arch_copy_{from,to}_user doesn't disable
pre-emption, and neither does its caller.

Pages can be unmapped just due to background paging.  I.e. it's a
normal occurrence, it doesn't require anything contrived.

So I think you're right: that looks like a bug.

The ARM uaccess code was written before CONFIG_PREEMPT was added, and
this couldn't happen then.  It could panic a kernel now.  I wonder why
it hasn't been noticed.  Maybe nobody turns on CONFIG_PREEMPT on ARM?

-- Jamie

Re: A question about PROT_NONE on ARM and ARM26

[Scott Wood]

On Fri, Jul 02, 2004 at 12:53:54AM +0100, Jamie Lokier wrote:
> The ARM uaccess code was written before CONFIG_PREEMPT was added, and
> this couldn't happen then.  It could panic a kernel now.  I wonder why
> it hasn't been noticed.  Maybe nobody turns on CONFIG_PREEMPT on ARM?

Given that such behavior implies some raciness in userspace, you'd
probably need either malicious or buggy user code to trigger it, and
in the latter case, you're limited to apps using threads.  Thus, it's
probably not that surprising that it hasn't been seen.

It could also happen in other rare cases, such as if the page gets
swapped out, and you get an I/O error swapping it back in, or if
forced filesystem unmounting were implemented.

-Scott