Page MenuHomeFreeBSD

Fix a use-after-free in pmap_release().
ClosedPublic

Authored by markj on Jan 2 2019, 6:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 7, 11:57 PM
Unknown Object (File)
Dec 20 2023, 1:22 AM
Unknown Object (File)
Nov 7 2023, 7:36 AM
Unknown Object (File)
Nov 5 2023, 12:27 AM
Unknown Object (File)
Oct 30 2023, 3:27 PM
Unknown Object (File)
Oct 6 2023, 6:30 AM
Unknown Object (File)
Oct 4 2023, 12:35 AM
Unknown Object (File)
Sep 16 2023, 3:40 AM
Subscribers

Details

Summary
  • Don't zero the page after freeing it.
  • Don't zero the page before removing it from allpmaps.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 21809
Build 21077: arc lint + arc unit

Event Timeline

sys/riscv/riscv/pmap.c
1306

Why zeroing it at all ?

One weird oddity. mips uses vm_page_free_zero() for it's top-level page but it doesn't zero it first. arm64 does the same. 32-bit arm v6 just leaks the pages (???), i386 uses vm_page_free() instead of vm_page_free_zero(). sparc64 uses vm_page_free_zero() without zero'ing the pages.

So it seems like mips, arm64, and sparc64 should all use vm_page_free() instead of vm_page_free_zero(). risc-v could also just do that instead of doing an explicit bzero here as well perhaps?

This revision is now accepted and ready to land.Jan 2 2019, 11:42 PM
In D18720#399435, @jhb wrote:

One weird oddity. mips uses vm_page_free_zero() for it's top-level page but it doesn't zero it first. arm64 does the same. 32-bit arm v6 just leaks the pages (???), i386 uses vm_page_free() instead of vm_page_free_zero(). sparc64 uses vm_page_free_zero() without zero'ing the pages.

So it seems like mips, arm64, and sparc64 should all use vm_page_free() instead of vm_page_free_zero(). risc-v could also just do that instead of doing an explicit bzero here as well perhaps?

I believe that the arches that use vm_page_free_zero() without zeroing, rely on the fact that all ptes are cleared before pmap is released. I.e. the page must be zero at this point. At least on arm64 vm_page_free() should be able to assert that the page is zeroed if DIAGNOSTIC is enabled.

And from the first hand experience, if the non-zero page returned to the free pool with PG_ZERO flag set, things breaks very fast, so it is visible, even if in puzzling manner.

sys/riscv/riscv/pmap.c
1306

Hmm, I think there's no reason. I will change it.

markj marked an inline comment as done.
  • Don't bother zeroing the top-level page.
This revision now requires review to proceed.Jan 3 2019, 12:06 AM
This revision is now accepted and ready to land.Jan 3 2019, 12:19 AM
This revision was automatically updated to reflect the committed changes.