Page MenuHomeFreeBSD

pmap: Avoid clearing the accessed bit for wired mappings
AcceptedPublic

Authored by markj on Fri, Mar 21, 2:09 PM.

Details

Reviewers
alc
kib
Summary

Currently pmap_advise() does not handle wired mappings specially in the
4KB mapping case: it will clear the accessed bit even though wired pages
cannot be reclaimed. This is mostly harmless in principle, but it can
violate an invariant in pmap_demote_pde_locked(), which assumes that a
superpage mapping with the accessed bit clear must not be wired. When
it encounters such a superpage, the mapping is deleted instead of being
demoted, but this is not permitted for wired mappings.

Since pmap_advise() can reasonably avoid clearing the accessed bit for
wired mappings, let's do that instead of adding additional complexity to
the promotion and demotion paths. For instance, one alternate solution
would be to have pmap_promote_pde() always set PG_A on a wired superpage
mapping, even when constituent PTEs do not all have it set, but I'm not
sure that that's preferable.

I believe the same problem exists on arm64 and will update the diff once
there's consensus on the right solution.

Reported by: syzbot+4b9dad11826c30bb6745@syzkaller.appspotmail.com

Diff Detail

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

Event Timeline

markj requested review of this revision.Fri, Mar 21, 2:09 PM
kib added inline comments.
sys/amd64/amd64/pmap.c
9317

I suspect that the new code would be clearer if you slightly restructure it:

   			if ((*pte & (PG_MANAGED | PG_V)) != (PG_MANAGED | PG_V))
				goto maybe_invlrng;
			else if ((*pte & (PG_M | PG_RW)) == (PG_M | PG_RW)) {
				if (advice == MADV_DONTNEED) {
					/*
					 * Future calls to pmap_is_modified()
					 * can be avoided by making the page
					 * dirty now.
					 */
					m = PHYS_TO_VM_PAGE(*pte & PG_FRAME);
					vm_page_dirty(m);
				}
			} else if ((*pte & PG_A) == 0)
				goto maybe_invlrng;
                        atomic_clear_long(pte, PG_M | ((*pte & PG_W) == 0 ? PG_A : 0));
This revision is now accepted and ready to land.Fri, Mar 21, 11:17 PM