Re: msync() bug

From: Andrea Arcangeli (andrea@suse.de)
Date: Mon Jul 09 2001 - 09:21:31 EST


On Mon, Jul 09, 2001 at 10:32:11PM +1000, Andrew Morton wrote:
> Abraham vd Merwe wrote:
> >
> > Hi!
> >
> > I was preparing some lecture last night and stumbled onto this bug. Maybe
> > some of you can shed some light on it.
> >
> > Basically, I just memory map /dev/mem at 0xb8000 (text mode - yes I know you
> > shouldn't do this, but it was to illustrate something), reads 4k, changes it
> > writes it back.
> >
>
> The actual call trace is:
>
> __set_page_dirty
> filemap_sync_pte
> filemap_sync_pte_range
> filemap_sync_pmd_range
> filemap_sync
> msync_interval
> sys_msync
>
> We're crashing because __set_page_dirty dereferences page->mapping,
> but pages from a mmap() of /dev/mem seem to have a NULL ->mapping.
>
> One of the very frustrating things about Linux kernel development
> is that the main source of tuition is merely the source code. You
> can stare at that for months (as I have) and still not have a firm
> grasp on the big-picture semantic *meaning* behind something as
> simple as a page having a null ->mapping. Sigh.
>
> So one is reduced to mimicry:
>
> --- linux-2.4.7-pre3/mm/filemap.c Wed Jul 4 18:21:32 2001
> +++ linux-akpm/mm/filemap.c Mon Jul 9 22:22:46 2001
> @@ -1652,7 +1652,8 @@ static inline int filemap_sync_pte(pte_t
> if (pte_present(pte) && ptep_test_and_clear_dirty(ptep)) {
> struct page *page = pte_page(pte);
> flush_tlb_page(vma, address);
> - set_page_dirty(page);
> + if (page->mapping)
> + set_page_dirty(page);
> }
> return 0;
> }

Wrong fix, `page' is just garbage if some non memory was mapped in
userspace (like framebuffers or similar mmio regions were mapped etc..).

I fixed it right ages ago (also sumbitted to Linus but got not
integrated and I forgotten to resend):

        ftp://ftp.us.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.7pre3aa1/00_msync-fb0-1

Please Linus include this time:

--- 2.4.5pre1aa3/mm/filemap.c.~1~ Fri May 11 02:08:28 2001
+++ 2.4.5pre1aa3/mm/filemap.c Mon May 14 18:48:59 2001
@@ -1808,10 +1808,12 @@
 {
         pte_t pte = *ptep;
 
- if (pte_present(pte) && ptep_test_and_clear_dirty(ptep)) {
+ if (pte_present(pte)) {
                 struct page *page = pte_page(pte);
- flush_tlb_page(vma, address);
- set_page_dirty(page);
+ if (VALID_PAGE(page) && !PageReserved(page) && ptep_test_and_clear_dirty(ptep)) {
+ flush_tlb_page(vma, address);
+ set_page_dirty(page);
+ }
         }
         return 0;
 }

Andrea
-
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/



This archive was generated by hypermail 2b29 : Sun Jul 15 2001 - 21:00:09 EST