Page MenuHomeFreeBSD

Several bug fixes and robustness improvements for the AP boot page table allocation.
ClosedPublic

Authored by kib on Aug 26 2018, 1:04 PM.

Details

Summary

At the time that mp_bootaddress() is called, phys_avail[] array does not reflect some memory reservations already done, like kernel placement. Recent changes to DMAP protection which make kernel text read-only in DMAP revealed this, where on some machines AP boot page tables selection appears to intersect with the kernel itself.

Fix this by checking the addresses selected using the same algorithm as bootaddr_rwx(). Also, try to chomp pages for the page table not only at the start of the contiguous range, but also at the end. This should improve robustness when the only suitable range is already consumed by the kernel.

N.B. I think that phys_avail[] manipulation functions from D16719 can be usefully applied there. At least, this code is very good testing place for that revision.

Reported and tested by: Michael Gmelin <freebsd@grem.de>

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib created this revision.Aug 26 2018, 1:04 PM
grembo accepted this revision.Aug 27 2018, 11:24 AM

Accepting to confirm that I tested this patch one more time against r338318 on the Acer Chromebook c720 (previous test was against an older version of the kernel). Obviously my feedback isn't sufficient and this still requires review by the original reviewers (aka experts).

This revision is now accepted and ready to land.Aug 27 2018, 11:24 AM
jhb added inline comments.Aug 27 2018, 4:25 PM
sys/amd64/amd64/mp_machdep.c
149 ↗(On Diff #47308)

Hmm, the 'end' here seems overly conservative. It seems like you kind of want something like 'end = start + AP_BOOTPT_SZ' as you do below for the "chomp at the end". You would have to add the same type of overflow check ('end <= physmap[i + 1]'). (That is, suppose you had some region much larger than 3 page and the start was ok, but 'end' wasn't for some reason, you would skip it.)

kib added inline comments.Aug 27 2018, 4:35 PM
sys/amd64/amd64/mp_machdep.c
149 ↗(On Diff #47308)

I see. Then is_mpboot_good() does not need end >= start + AP_BOOT_SZ check, which brings back the symmetry.

kib updated this revision to Diff 47350.Aug 27 2018, 4:43 PM
kib marked an inline comment as done.

When chomping at start, only check that start and start + 3 pages do not fall into kernel memory.
Also check for overflows.

This revision now requires review to proceed.Aug 27 2018, 4:43 PM
jhb accepted this revision.Aug 27 2018, 5:19 PM
This revision is now accepted and ready to land.Aug 27 2018, 5:19 PM
grembo accepted this revision.Aug 28 2018, 12:40 PM

Tested again, new version also works as expected, mptramp_pagetables is 0x7bf77000 on c720

This revision was automatically updated to reflect the committed changes.