[lkml]   [2010]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: Ugly rmap NULL ptr deref oopsie on hibernate (was Linux 2.6.34-rc3)
    Hi, Linus. 

    On Tue, 2010-04-06 at 08:55 -0700, Linus Torvalds wrote:
    > On Wed, 7 Apr 2010, Minchan Kim wrote:
    > >
    > > Let's see the unlink_anon_vmas.
    > >
    > > 1. list_for_each_entry_safe(avc,next, vma->anon_vma_chain, same_vma)
    > > 2. anon_vma_unlink
    > > 3. spin_lock(anon_vma->lock) <-- HERE LOCK.
    > > 4. list_del(anon_vma_chain->same_anon_vma);
    > >
    > > What if anon_vma is destroyed and reuse by SLAB_XXX_RCU for another
    > > anon_vma object between 2 and 3?
    > > I mean how to make sure 3) does lock valid anon_vma?
    > >
    > > I hope it is culprit.
    > I don't think so. That isn't the racy case. We're working with a
    > anon_vma_chain, so the anonvma is all there.

    But the anon_vma is using for another anon_vma.
    Nonetheless, anon_vma_unlink does list_del(anon_vma's same_anon_vma).
    I doubt it.

    > The racy case is when we look up an anonvma by the page, and the page gets
    > unmapped at the same time because somebody else is travelling over the LRU
    > list of the page itself, isn't it?

    Yes. but I thought page might travel with anon_vmas which have
    same_anon_vma deleted by race.

    > I do wonder if "page_lock_anon_vma()" should check the whole
    > "page_mapped()" case _after_ taking the anon_vma lock. Because if the race
    > happens, we're following a anon_vma list that has nothing to do with that
    > page (it's stilla _valid_ list, since we locked the anon_vma, but will it
    > be ok?)

    So we always use it with (vma_address and page_check_address) to make
    sure validation of anon_vma.
    But I think it's not good design. I want to hold lock ahead checking of
    page_mapped but maybe performance issue? I am not sure.

    > IOW, what is it that really keeps the anon_vma list reliable _and_
    > relevant wrt the page? We know we may get a stale anon_vma, are we ok if
    > that anon_vma list doesn't actually have anything to do with the page any
    > more?
    > I think the first check in "page_address_in_vma()" protects us, but
    > whatever.
    > However, that made me look at the PAGE_MIGRATION case. That seems to be
    > just broken. It's doing that page_anon_vma() + spin_lock without holding
    > any RCU locks, so there is no guarantee that anon_vma there is at all
    > valid.

    FYI, recently there is a patch about migration case.

    > Is that function always called with rcu_read_lock()?
    > Linus

    Kind regards,
    Minchan Kim

     \ /
      Last update: 2010-04-06 18:27    [W:0.035 / U:151.672 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site