Page MenuHomeFreeBSD

powerpc/radix: acquire the pmap lock in mmu_radix_extract()
AcceptedPublic

Authored by pkubaj on Wed, Sep 2, 9:55 AM.
Tags
None
Referenced Files
F169842040: D59320.id185619.diff
Wed, Sep 2, 5:46 PM
F169841935: D59320.id185619.diff
Wed, Sep 2, 5:45 PM
F169833713: D59320.diff
Wed, Sep 2, 5:15 PM
F169819044: D59320.id185619.diff
Wed, Sep 2, 4:09 PM
F169817873: D59320.diff
Wed, Sep 2, 3:57 PM
F169771558: D59320.diff
Wed, Sep 2, 12:44 PM
F169758283: D59320.id185619.diff
Wed, Sep 2, 11:39 AM
F169755459: D59320.id.diff
Wed, Sep 2, 11:29 AM
Subscribers

Details

Reviewers
markj
Group Reviewers
PowerPC
Summary

mmu_radix_extract() walks the page tables without holding the pmap lock,
unlike its hash MMU counterpart moea64_extract(). A concurrent unmap can
free and recycle the page table page being walked, so the read returns
whatever now occupies that memory and the caller gets a physical address
that never existed.

That is how mmu_radix_sync_icache() came to hand a bogus address to
__syncicache() and panic the machine. Commit 1574ca1955f5 worked around
it by taking the pmap lock in mmu_radix_sync_icache(), but the machine
independent callers of pmap_extract() - vm_sync_icache(), proc_rwmem()
and the vslock() paths - remain exposed to the same failure.

Rename the existing body to mmu_radix_extract_locked(), which asserts the
lock, and make mmu_radix_extract() a thin wrapper that acquires it.
mmu_radix_sync_icache() already holds the pmap lock, so it calls the
locked variant directly and neither recurses nor reacquires the lock once
per page.

Suggested by: alc

MFC after: 1 week

Test Plan

POWER9 (radix MMU), GENERIC64LE kernel built from main.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 76404
Build 73287: arc lint + arc unit

Event Timeline

pkubaj requested review of this revision.Wed, Sep 2, 9:55 AM
markj added a subscriber: markj.
markj added inline comments.
sys/powerpc/aim/mmu_radix.c
3508

BTW, this line is redundant, the previous line is already ORing in that value.

3523

This too.

This revision is now accepted and ready to land.Wed, Sep 2, 12:59 PM