Page MenuHomeFreeBSD

Handle overflow in calculating max kmem size.
ClosedPublic

Authored by kib on Mon, Jan 7, 4:00 AM.

Details

Summary

vm_kmem_size is u_long, and it might be not capable of holding page count times PAGE_SIZE, even when scaled down by VM_KMEM_SIZE_SCALE. As bde reported, 12G PAE config ends up with zero for kmem size.

Explicitly check for overflow and clamp kmem size at vm_kmem_size_max. If we end up at zero size because VM_KMEM_SIZE_MAX is not defined, panic with clear explanation rather then failing in weird way.

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.Mon, Jan 7, 4:00 AM
kib added a subscriber: pho.Mon, Jan 7, 4:00 AM
markj added inline comments.Mon, Jan 7, 5:57 AM
sys/kern/kern_malloc.c
935 ↗(On Diff #52618)

Why does vm_kmem_size * PAGE_SIZE < vm_kmem_size imply that the overflowed value is 0?

Also, if vm_kmem_size_max == 0 and vm_kmem_size_min != 0, the code above may just silently set vm_kmem_size = vm_kmem_size_min in the overflow case. Shouldn't we panic in that case too?

kib added inline comments.Mon, Jan 7, 6:49 AM
sys/kern/kern_malloc.c
935 ↗(On Diff #52618)

I only want to panic if the value for the vm_kmem_size is insane. If there are some defined values for VM_KMEM_SIZE_MIN/MAX, and they are used, this should be fine. Even if only _MIN is defined and used.

So no, the overflow does not imply that the overflown value is zero, it only means that we fall back to vm_kmem_size_max, whatever it is (except zero).

markj added inline comments.Tue, Jan 8, 12:30 AM
sys/kern/kern_malloc.c
935 ↗(On Diff #52618)

But if there is an overflow and vm_kmem_size_max == vm_kmem_size_min == 0, we may set vm_kmem_size to a non-zero insane value.

kib updated this revision to Diff 52654.Tue, Jan 8, 1:05 AM

Do not that vm_kmem_size_max > 0 when handling overflow.
We will panic later if it is 0.

markj accepted this revision.Tue, Jan 8, 1:07 AM
This revision is now accepted and ready to land.Tue, Jan 8, 1:07 AM
This revision was automatically updated to reflect the committed changes.