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

[markj]

Summary

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

[kib]

sys/riscv/riscv/pmap.c

1306 ↗

(On Diff #52488)

Why zeroing it at all ?

[jhb]

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?

[kib]

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.

[markj]

sys/riscv/riscv/pmap.c

1306 ↗

(On Diff #52488)

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

[markj]

• Don't bother zeroing the top-level page.