Messages in this thread Patch in this message | | | Date | Tue, 25 May 2004 05:43:26 +0200 | From | Andrea Arcangeli <> | Subject | Re: [PATCH] ppc64: Fix possible race with set_pte on a present PTE |
| |
On Sun, May 23, 2004 at 10:10:16PM -0700, Linus Torvalds wrote: > what you found is actually a (really really) unlikely race condition that > can have serious consequences.
agreed, it could in theory lose the dirty bit.
> course, back then SMP wasn't an issue, and this seems to have survived all > the SMP fixes.
looks like to trigger this two threads needs to page fault on the same not-present MAP_SHARED page at the same time and the second thread must be a read-fault. The first thread will establish the pte writeable and dirty, then the first thread releases the page_table_lock, then the VM takes the page_table_lock (from kswapd or similar) then the VM transfers the dirty bit from pte to page and it even starts the and completes the I/O before the the read-fault has a chance to execute. Then after the I/O is completed the second thread finally takes the page_table_lock and reads the pte. Then the first thread writes to the page from userspace marking the pte dirty , and finally the second thread completes the page_fault running pte_establish and losing the dirty bit on the page.
Maybe it can trigger even without I/O in between, not sure, certainly it can happen in the above scenario, but the I/O window is quite huge for not being able to take a spinlock before the I/O is started.
The below patch should fix it, the only problem is that it can screwup some arch that might use page-faults to keep track of the accessed bit, I think for istance ia64 might do that, not sure if it's using it in linux though. anyways the below should be the optimal fix for x86 and x86-64 at least and the other archs will not corrupt memory anymore too (at worst they will live lock in userspace, and the livelock should be solvable gracefully with sigkill since it's an userspace one).
warning, patch is absolutely untested.
Index: mm/memory.c =================================================================== RCS file: /home/andrea/crypto/cvs/linux-2.5/mm/memory.c,v retrieving revision 1.154 diff -u -p -r1.154 memory.c --- mm/memory.c 20 Apr 2004 14:22:03 -0000 1.154 +++ mm/memory.c 25 May 2004 03:04:48 -0000 @@ -1665,10 +1665,30 @@ static inline int handle_pte_fault(struc return do_wp_page(mm, vma, address, pte, pmd, entry); entry = pte_mkdirty(entry); + + /* + * We can overwrite the pte not atomically only if this is a + * write fault or we can lose the dirty bit. The dirty and the + * accessed bit are the only two things that can change form + * under us even if we hold the page_table_lock, they're + * set by hardware while other threads runs in userspace. + * + * This could cause an infinite loop with a read-fault if some + * arch tries to keep track of the accessed bit with a kernel + * page fault (but keeping track of the accessed bit with + * kernel exceptions sounds very inefficient anyways). + * If some arch infinite loops, they will have to implement + * some sort of atomic ptep_stablish where they atomically + * compare and swap, then we'll change the common code here + * to learn how to atomic swap the pte. + * + * x86 and x86-64 are sure optimal without atomic ops here and + * with this simple code. + */ + entry = pte_mkyoung(entry); + ptep_establish(vma, address, pte, entry); + update_mmu_cache(vma, address, entry); } - entry = pte_mkyoung(entry); - ptep_establish(vma, address, pte, entry); - update_mmu_cache(vma, address, entry); pte_unmap(pte); spin_unlock(&mm->page_table_lock); return VM_FAULT_MINOR; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |