bugs in contigmalloc*() related to "page not found in hash" panics

From: Matthew Dillon (dillon_at_apollo.backplane.com)
Date: 11/10/04

  • Next message: Xin LI: "Re: Is there any way to know if userland is patched?"
    Date: Wed, 10 Nov 2004 10:01:28 -0800 (PST)
    To: freebsd-hackers@freebsd.org
    
    

        I've tracked down several bugs in contigmalloc*() in DragonFly based on
        kernel cores provided by David Rhodus. These bugs have just been addressed
        in DFly but also need to be addressed in FreeBSD-4, and at least some
        work must also be done in FreeBSD-5/6. So someone needs to take up
        the ball and deal with this in FreeBSD.

        Here is the DragonFly commit.

        http://www.dragonflybsd.org/cvsweb/src/sys/vm/vm_contig.c.diff?r1=1.10&r2=1.11&f=u

        FreeBSD-4:

            FreeBSD-4 is in the same situation that DFly was in and requires the
            same fixes as the above patch, though note that in FreeBSD-4 the
            contigmalloc() code is in vm_page.c, not vm_contig.c.

        FreeBSD-5/6:

            FreeBSD-5/6 may also has issues, though it is likely that they are
            masked by later checks done in FreeBSD-5/6's contigmalloc code.

            The main issue is the attempt to reuse a PQ_CACHE page by
            unconditionally vm_page_free()ing it without checking to see if
            it is held, busied, or anything else. This sounds a bit dangerous
            to me since vm_page_select_cache() does NOT do this.

            I recommend that FreeBSD-5/6's contigmalloc code be adjusted similar
            to vm_page_select_cache() and not try to free cache pages which
            are held or busied or otherwise in a weird state.

            FreeBSD-5/6 *DOES* recheck that the pages are PQ_FREE before it
            actually reallocates them, and this may be its saving grace (since
            vm_page_free() might have moved the page to PQ_HOLD this recheck
            should catch the incorrect cache->free cases). But it is unclear
            whether the PQ_CACHE -> vm_page_free() code above was itself safe
            so it may be better to be safe then sorry.

            You also need to replace the unconditional 'm->object = NULL' with
            an assertion that m->object is already NULL, because if m->object
            isn't NULL it means that the page is in the VM page bucket hash table
            and NULLing out the object is guarenteed to corrupt the hash table.

        Note in particular the symptoms of the contigmalloc() problem:

        * "page not found in hash" panics (due to VM bucket hash table corruption).

        * Memory corruption, particularly filesystem corruption since those are
          the most likely pages to be in a held state while on the free list.
          Due to a page being reused before it is reusable.

        If you have either of these two symptoms in FreeBSD-5/6 then you need
        to take a very close look at your contigmalloc() code, even if you think
        it is correct.

                                            -Matt
                                            Matthew Dillon
                                            <dillon@backplane.com>
    _______________________________________________
    freebsd-hackers@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
    To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"


  • Next message: Xin LI: "Re: Is there any way to know if userland is patched?"