Messages in this thread | | | Date | Mon, 12 Apr 1999 16:39:02 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [patch] fix for cc1 Out of memory [Re: [TESTCASE] `Out of memory for cc1'] |
| |
On Tue, 13 Apr 1999, Andrea Arcangeli wrote: > > Obviously this pte-not-zeroed page can be returned only from the > pagetable-cache.
Right, that was my suspicion too..
> After a little grep: > > andrea@laser:/usr/src/linux/mm$ grep clear_page_tables *.c > memory.c:void clear_page_tables(struct mm_struct *mm, unsigned long first, > int nr) > memory.c: * clear_page_tables(). > mmap.c: clear_page_tables(mm, first, last-first); > mmap.c: clear_page_tables(mm, 0, USER_PTRS_PER_PGD); > > I seen that the only piece of code that are recalling clear_page_tables() > are exit_mmap() and free_pgtables().
Indeed. But also notice how free_pgtables() only calls it on page tables that should have been individually zapped for all mapped areas with "zap_page_range()" for the whole mapped area. As such, if there are page tables in the free lists that haven't been cleared, then there is a real bug somewhere else.
That's why I still maintain that your patch just hides the _real_ bug, which is that something continues to use the page tables after the area has been zapped (or possibly something fills in the page tables _beyond_ the area that the mapping was for, which would also explain why the page table zapping didn't work).
Note that if we have page tables with stale data, that's a security problem. And it mustn't be just hidden by making sure that we clear the page tables at some later date .. (your patch makes sure that we clear the page directory entries before we free them, but it still means that in the meantime the entries have some stale stuff that we don't have a vma for, see?)
So now do you wonder why I was looking at the shm code? The stuff I removed was largely the code that was initializing and playing with the page tables directly.. Exactly the kind of thing that might cause something like this if it wasn't careful enough.
Which is why I'd like people to test out my patch (I just verified that it at least runs doom, so while it is probably broken, it's at least not _completely_ broken, and that makes me happier about it all).
> So my conclusing is been that free_pgtables() was calling > clear_page_tables on not-yet-cleared pte.
I agree with your conclusion. I just disagree with your patch ;)
> So I thought a bit about the issue and the `first' and `last' variable of > free_pgtables() looked buggy to me. > > My point is that zap_page_range run (in the past) starting from A to B. If > then you call clear_page_tables() starting from `A & PGDIR_MASK' to `B + > PGDIR_SIZE - 1', you are going to call pte_free() also on pagetables that > are not been previously cleared by zap_page_range (outside the range > A-B)...
Sure. It's _designed_ to clear out extra pgtable entries that can cover areas outside the range A-B. That's the whole idea.
But they had better have been cleared already: the whole point with the prev and next checking is to make sure that there are no active mappings in the area, and that in turn means that they had better have been cleared by zap_page_range() earlier. See my point?
> If this free_pgtables() aggressive-behaviour is completly safe I would > like to understand why though ;). And if there are leaking-memory testcase > (Alan?) I would like to take a look to understand better why we need such > aggressive-behavior ;))). Thanks.
Think about it. free_pgtables() only gets rid of page directories that have no mappings, and as such they must be clear. No?
Think something on the order of (written on-the-fly, I won't guarantee this is correct, but I think you'll get the idea).
char *base = (char *)4096; for (;;) { base = mmap(base, 4096, PROT_READ, MAP_PRIVATE | MAP_ANON, -1, 0); if (-1 == (int) base) { perror("mmap"); sleep(100); exit(0); } munmap(base, 4096); base += 4096; }
and think about what happens to the page tables with a plain 2.2.5 and with your patches..
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.tux.org/lkml/
| |