Page MenuHomeFreeBSD

Fix detection of kernel memory overlap

Authored by on Apr 17 2018, 7:47 PM.



Function moea64_early_bootstrap was not detecting overlaps
between available and kernel memory regions when the
available region was entirely contained in kernel region.

This would result in kernel memory being considered as
available for other purposes, corrupting kernel memory

The fix consists in adding an additional check and exclude
memory regions inside [kernelstart, kernelend] range.

Diff Detail

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

Event Timeline

jhibbits added inline comments.Apr 17 2018, 8:10 PM
705 ↗(On Diff #41592)

Style nit: return values should be wrapped in ()'s (yes, we violate that in a few spots here, but it's style). Won't penalize for it, though.

788 ↗(On Diff #41592)

Do you need the cast here? I think it's redundant, but may be wrong.

705 ↗(On Diff #41592)


788 ↗(On Diff #41592)

Without the cast it's harder to be sure that it will be correct for every platform.

If phys_avail type will always be uint64_t [] in this code, than it should be safe to use either ~0UL or ~0ULL.

But if it may be uint32_t, than using 0U and (possibly) widening it to vm_paddr_t before negating it would be better.

~0ULL should also work for uint32_t, as ~0ULL will probably be truncated to 32 bits in this case. But the compiler may produce a warning about narrowing the value.

What do you think?

jhibbits added inline comments.Apr 17 2018, 9:11 PM
788 ↗(On Diff #41592)

~0 should suffice. We use that in other places to represent all-one values, such as in bus_alloc_resource_any. C rules will automagically return the correct value.

  • Addressing review's comments marked 5 inline comments as done.Apr 17 2018, 9:21 PM
jhibbits accepted this revision.Apr 18 2018, 3:20 PM

Great work!

This revision is now accepted and ready to land.Apr 18 2018, 3:20 PM

Thanks @jhibbits! Can you commit the change for me?

swills added a subscriber: swills.Apr 19 2018, 6:02 PM

Booted r331763 with this change and it fixed the problem for me.

This revision was automatically updated to reflect the committed changes.