Re: Progress! was: Re: Yet more VM writable swap-cached pages

Re: Progress! was: Re: Yet more VM writable swap-cached pages

[Linus Torvalds]

On Fri, 10 Jul 1998, Ganesh Sittampalam wrote:
> 
> Since this is now 100% reproducible on my system, I'd be happy to
> guinea-pig any fixes, however experimental.

Does this fix it dor you?

		Linus
-----
diff -u --recursive --new-file v2.1.108/linux/include/asm-i386/pgtable.h linux/include/asm-i386/pgtable.h
--- v2.1.108/linux/include/asm-i386/pgtable.h	Wed Jun 24 22:54:10 1998
+++ linux/include/asm-i386/pgtable.h	Thu Jul  9 18:13:50 1998
@@ -225,6 +225,9 @@
 #define _PAGE_4M	0x080	/* 4 MB page, Pentium+.. */
 #define _PAGE_GLOBAL	0x100	/* Global TLB entry PPro+ */
 
+#define _PAGE_READABLE  (_PAGE_PRESENT)             
+#define _PAGE_WRITABLE  (_PAGE_PRESENT | _PAGE_RW)
+
 #define _PAGE_TABLE	(_PAGE_PRESENT | _PAGE_RW | _PAGE_USER | _PAGE_ACCESSED | _PAGE_DIRTY)
 #define _KERNPG_TABLE	(_PAGE_PRESENT | _PAGE_RW | _PAGE_ACCESSED | _PAGE_DIRTY)
 #define _PAGE_CHG_MASK	(PAGE_MASK | _PAGE_ACCESSED | _PAGE_DIRTY)
@@ -330,21 +333,25 @@
  * Undefined behaviour if not..
  */
 extern inline int pte_read(pte_t pte)		{ return pte_val(pte) & _PAGE_USER; }
-extern inline int pte_write(pte_t pte)		{ return pte_val(pte) & _PAGE_RW; }
 extern inline int pte_exec(pte_t pte)		{ return pte_val(pte) & _PAGE_USER; }
 extern inline int pte_dirty(pte_t pte)		{ return pte_val(pte) & _PAGE_DIRTY; }
 extern inline int pte_young(pte_t pte)		{ return pte_val(pte) & _PAGE_ACCESSED; }
 
-extern inline pte_t pte_wrprotect(pte_t pte)	{ pte_val(pte) &= ~_PAGE_RW; return pte; }
 extern inline pte_t pte_rdprotect(pte_t pte)	{ pte_val(pte) &= ~_PAGE_USER; return pte; }
 extern inline pte_t pte_exprotect(pte_t pte)	{ pte_val(pte) &= ~_PAGE_USER; return pte; }
 extern inline pte_t pte_mkclean(pte_t pte)	{ pte_val(pte) &= ~_PAGE_DIRTY; return pte; }
 extern inline pte_t pte_mkold(pte_t pte)	{ pte_val(pte) &= ~_PAGE_ACCESSED; return pte; }
-extern inline pte_t pte_mkwrite(pte_t pte)	{ pte_val(pte) |= _PAGE_RW; return pte; }
 extern inline pte_t pte_mkread(pte_t pte)	{ pte_val(pte) |= _PAGE_USER; return pte; }
 extern inline pte_t pte_mkexec(pte_t pte)	{ pte_val(pte) |= _PAGE_USER; return pte; }
 extern inline pte_t pte_mkdirty(pte_t pte)	{ pte_val(pte) |= _PAGE_DIRTY; return pte; }
 extern inline pte_t pte_mkyoung(pte_t pte)	{ pte_val(pte) |= _PAGE_ACCESSED; return pte; }
+
+/*
+ * These are harder, as writability is two bits, not one..
+ */
+extern inline int pte_write(pte_t pte)		{ return (pte_val(pte) & _PAGE_WRITABLE) == _PAGE_WRITABLE; }
+extern inline pte_t pte_wrprotect(pte_t pte)	{ pte_val(pte) &= ~((pte_val(pte) & _PAGE_PRESENT) << 1); return pte; }
+extern inline pte_t pte_mkwrite(pte_t pte)	{ pte_val(pte) |= _PAGE_RW; return pte; }
 
 /*
  * Conversion functions: convert a page and protection to a page entry,



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu

Re: Progress! was: Re: Yet more VM writable swap-cached pages

[Bill Hawes]

Linus Torvalds wrote:

> -extern inline pte_t pte_wrprotect(pte_t pte)   { pte_val(pte) &= ~_PAGE_RW; return pte; }
>  extern inline pte_t pte_rdprotect(pte_t pte)   { pte_val(pte) &= ~_PAGE_USER; return pte; }
>  extern inline pte_t pte_exprotect(pte_t pte)   { pte_val(pte) &= ~_PAGE_USER; return pte; }
>  extern inline pte_t pte_mkclean(pte_t pte)     { pte_val(pte) &= ~_PAGE_DIRTY; return pte; }
>  extern inline pte_t pte_mkold(pte_t pte)       { pte_val(pte) &= ~_PAGE_ACCESSED; return pte; }
> -extern inline pte_t pte_mkwrite(pte_t pte)     { pte_val(pte) |= _PAGE_RW; return pte; }
>  extern inline pte_t pte_mkread(pte_t pte)      { pte_val(pte) |= _PAGE_USER; return pte; }
>  extern inline pte_t pte_mkexec(pte_t pte)      { pte_val(pte) |= _PAGE_USER; return pte; }
>  extern inline pte_t pte_mkdirty(pte_t pte)     { pte_val(pte) |= _PAGE_DIRTY; return pte; }
>  extern inline pte_t pte_mkyoung(pte_t pte)     { pte_val(pte) |= _PAGE_ACCESSED; return pte; }
> +
> +/*
> + * These are harder, as writability is two bits, not one..
> + */
> +extern inline int pte_write(pte_t pte)         { return (pte_val(pte) & _PAGE_WRITABLE) == _PAGE_WRITABLE; }
> +extern inline pte_t pte_wrprotect(pte_t pte)   { pte_val(pte) &= ~((pte_val(pte) & _PAGE_PRESENT) << 1); return pte; }
> +extern inline pte_t pte_mkwrite(pte_t pte)     { pte_val(pte) |= _PAGE_RW; return pte; }
                                                                    ^^^^^^^^
Did you mean to put in _PAGE_WRITABLE here, or can the pte_mkwrite macro always assume the
PRESENT bit is already set?

Regards,
Bill

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu

Re: Progress! was: Re: Yet more VM writable swap-cached pages

[Linus Torvalds]

On Thu, 9 Jul 1998, Bill Hawes wrote:
>
> > +extern inline pte_t pte_mkwrite(pte_t pte)     { pte_val(pte) |= _PAGE_RW; return pte; }
>                                                                     ^^^^^^^^
> Did you mean to put in _PAGE_WRITABLE here, or can the pte_mkwrite macro always assume the
> PRESENT bit is already set?

I decided that it cannot matter. If somebody tries to make a PROT_NONE
page writable by using pte_mkwrite(), there is already a bug there, and
I'm happier keeping it PROT_NONE than I am to mark it present. 

Note that this can not happen through a normal page fault, because a
normal page fault would have noticed that we don't actually have write
permission to the page at all. As such, the only way somebody can mark a
PROT_NONE page writable is if we're doing the nasty "bring in all the
pages because somebody did a mlock[all]() on us". 

In which case the above does the right thing, by certainly bringing the
page in, but not actually allowing anybody to read/write to it (actually,
I don't think this can happen even in that case, because if we have
PROT_NONE then the make_pages_present() stuff will not try to bring it
into memory writably, so we won't even try to make it writable). 

In short, the above really only makes sense if PRESENT is already set, and
if it was PROT_NONE from before it is a no-op which is fine.

		Linus


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu

Progress! was: Re: Yet more VM writable swap-cached pages

[Stephen C. Tweedie]

Hi,

On Thu, 9 Jul 1998 18:47:10 +0100 (BST), Ganesh Sittampalam
<ganesh.sittampalam@ox.compsoc.net> said:

> Just after that last e-mail, I got a flood of them.
> Jul  9 18:27:30 munchkin kernel: VM: Found a writable swap-cached page!
> Jul  9 18:27:30 munchkin kernel: pte   6a8042, vma flags 00000070, page
> flags 0000028c, count 2
> Jul  9 18:27:30 munchkin kernel: page=c0206f80@0017da00, found=c0206f80,
> count=3

Excellent!!!!!!!!!!!!!

vma flags represent a private vma with read/write/exec-privilege but
with no rwe currently enabled.  Page flags are normal for a resident
swap-cached anonymous page with no IO in flight.  The page count, 2, is
also normal for a cached anonymous page.  The pte, 6a8042, is not normal
at all.  It is marked non-present (the lowest bit is clear) but
_PAGE_PROTNONE.  That changes everything.

Thanks --- this tells us exactly what has gone wrong, I think.
Something, somewhere, (electric fence, perhaps?) has set up a region of
memory with no access allowed.  There is a page mapped, but it is not
visible to the process: somebody has done an mprotect() to eliminate the
visibility of the page.  That clears the _PAGE_PRESENT bit on the pte
but keeps the _PAGE_PROTNONE bit set, and _PAGE_PROTNONE is an alias for
_PAGE_RW!!  That's the trouble: the pte_write() test to see if a page is
writable tests the _PAGE_RW bit but fails to first of all check whether
or not _PAGE_PRESENT is set in the first place.


I've just tried to reproduce this with the program at the end, which
creates page of local memory, allows it to be swapped out, then pages it
in and marks it PROT_NONE.  I got a slightly different end result,
exactly the same, on two attempts out of two:

	 swap_free: Trying to free nonexistent swap-page

which also quite possibly results from the swap cache code seeing this
page as writable when it is not.  Anyway, it is now clear that we can
reproduce some rather undesirable behaviour using PROT_NONE, and the pte
trace from your own report also indicates that it may be the source of
your own problems.

I _think_ that on Intel we can fix much of this by correcting the macro

  extern inline int pte_write(pte_t pte) { return pte_val(pte) & _PAGE_RW; }

to check for (_PAGE_RW && !_PAGE_PRESENT).  I'm not entirely sure yet
that this will be the end of it; tomorrow I'll have a dig around to see
if I can find any other nasties which might trip us up here.  What are
the implications for other architectures which organise their ptes
differently?

--Stephen

----------------------------------------------------------------
Run this.  When it suspends itself, force it out to swap somehow then
bring the test program back with "%" at the shell.  It will suspend
itself again; at this point, the page should back in the swap cache and
protected PROT_NONE.  Things now go wrong.

protnone.c:

/*
 * Create a page of prot-none memory
 */

#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <sys/mman.h>

void try(const char *why, int error) 
{
	if (!error)
		return;
	perror(why);
	exit(1);
}

int main(void)
{
	int pagesize = getpagesize();
	char *page;
	volatile char a;
	
	try ("malloc", (page = malloc(2*pagesize)) == NULL);

	/* Round up to the next page boundary */
	page = (char *) (((unsigned long) page + pagesize-1) & ~(pagesize-1));
	
	/* Make a private page... */
	*page = 0;
	
	/* Give it a chance to get swapped out...  */
	kill (getpid(), SIGSTOP);

	/* Swap it back in (leaving it swap-cached of course!)... */
	a = *page;
	
	/* ... and map it prot-none. */
	try ("mprotect", mprotect (page, pagesize, PROT_NONE));

	/* Now, wait for the damage. */
	kill (getpid(), SIGSTOP);
	
	return 0;
}


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu

Re: Progress! was: Re: Yet more VM writable swap-cached pages

[Linus Torvalds]

On Fri, 10 Jul 1998, Stephen C. Tweedie wrote:
> with no rwe currently enabled.  Page flags are normal for a resident
> swap-cached anonymous page with no IO in flight.  The page count, 2, is
> also normal for a cached anonymous page.  The pte, 6a8042, is not normal
> at all.  It is marked non-present (the lowest bit is clear) but
> _PAGE_PROTNONE.  That changes everything.

Cool. This does indeed explain it.

The _PAGE_PROTNONE was a clever way to get the correct unreadability on an
x86, but I did indeed miss the fact that now a page can be marked
"present" as far as the Linux memory management is concerned, yet not be
writable by looking at _PAGE_RW. 

Your suggestion not only should fix this, but is also the RightThing(tm)
to do. 

It also explains why so few people saw this - PROT_NONE is not something
that is normally used.

>							  What are
> the implications for other architectures which organise their ptes
> differently?

Other architectures may have the same bug, but it's actually fairly
unlikely. Most other architectures tend to have a nicer way to do
PROT_NONE anyway, and the x86 thing is a hack (but a very nice hack,
because it leaves the mm layer completely unaware of the fact that the x86
page tables are fairly deficient in this area). 

So it's not a conceptual problem, it might just be something that needs to
be looked at. Certainly the alpha does not have this problem.

		Linus


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu

Re: Progress! was: Re: Yet more VM writable swap-cached pages

[Richard Henderson]

On Thu, Jul 09, 1998 at 05:54:21PM -0700, Linus Torvalds wrote:
> It also explains why so few people saw this - PROT_NONE is not something
> that is normally used.

Actually, the glibc ld.so will create PROT_NONE regions if there is
a hole between a shared library's text and data space.  E.g.

20000110000-200001d6000 r-xp 00000000000 08:02 29407      /lib/libc-2.0.7.so
200001d6000-200002d0000 ---p 000000c6000 08:02 29407      /lib/libc-2.0.7.so
200002d0000-200002e6000 rwxp 000000c0000 08:02 29407      /lib/libc-2.0.7.so

    LOAD off    0x00000000 vaddr 0x00000000 paddr 0x00000000 align 2**16
         filesz 0x000c48a8 memsz 0x000c48a8 flags r-x
    LOAD off    0x000c48a8 vaddr 0x001c48a8 paddr 0x001c48a8 align 2**16
         filesz 0x00010540 memsz 0x0001ae90 flags rwx


r~

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html

Re: Progress! was: Re: Yet more VM writable swap-cached pages

[Linus Torvalds]

On Mon, 20 Jul 1998, Richard Henderson wrote:
> On Thu, Jul 09, 1998 at 05:54:21PM -0700, Linus Torvalds wrote:
> > It also explains why so few people saw this - PROT_NONE is not something
> > that is normally used.
> 
> Actually, the glibc ld.so will create PROT_NONE regions if there is
> a hole between a shared library's text and data space.  E.g.
> 
> 20000110000-200001d6000 r-xp 00000000000 08:02 29407      /lib/libc-2.0.7.so
> 200001d6000-200002d0000 ---p 000000c6000 08:02 29407      /lib/libc-2.0.7.so
> 200002d0000-200002e6000 rwxp 000000c0000 08:02 29407      /lib/libc-2.0.7.so

Yes, but you won't have any actualy _pages_ mapped there. 

The only way to get the pages there is to first map it with something else
than prot_none, touch some of the pages, and then do a mprotect() to make
them invisible again. I doubt glibc does that ;) 

		Linus



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html

Re: Progress! was: Re: Yet more VM writable swap-cached pages

[Stephen C. Tweedie]

Hi,

On Thu, 09 Jul 1998 22:06:42 -0400, Bill Hawes <whawes@star.net> said:

> In my searches for the problem I had overlooked the interaction
> between the PRESENT and PROT_NONE bits.

You're not the only one!

> Hopefully any remaining swap bugs won't be so hard to track down ...

Yep.  Ingo and I have been doing a bunch of swapping stress tests with
no problems so far, so we're hoping that any other problems that turn
up will be due to the use of specific features like PROT_NONE and
won't be problems in the underlying swap mechanisms.  Well, we can
hope, can't we?  :)

--Stephen


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu