kern: osd: avoid dereferencing freed slots

[kevans]

Summary

If a slot is freed that isn't the last one, we'll set its destructor to
NULL to indicate that it's been freed and leave a hole in the slot map.
Check osd_destructors in osd_call() to avoid dereferencing a method that
is potentially from a module that's been unloaded.

While we're here, reduce the # slots as much as possible if we free the
last one. While we'll re-use unused slots anyways, if we're going to
bother reallocating at all then we might as well re-pack it as much as
we can.

[markj]

sys/kern/kern_osd.c

186

This assumption is wrong. I'm rather skeptical that it's useful to shrink the array at all.

[kevans]

sys/kern/kern_osd.c

186

More than happy to drop this part of the patch in a plan to do something else; I think it makes sense to reclaim freed slots sometimes so that we're not just ever-growing,, but if we're going to downsize it by just one entry when we have a contiguous block of freed slots at the end then that doesn't seem to be worth it at all.

OTOH, if we free up all of the slots at the end then we're going just to reallocate immediately on the next osd_register call for this type.

400

I meant to note that this is most easily reproduced by:

kldload if_wg
kldload mqueuefs
kldunload if_wg
# We just freed the slot right before mqueuefs
jail -c path=/ command=/bin/ls
# Panic as we try to call wg_prison_remove() after the module unloaded

[jamie]

The important part of this clearly good.

sys/kern/kern_osd.c

186

This assumption is wrong.

Indeed. I was surprised by this - it always seemed so common-sense to guarantee a smaller reallocation.

So this is going to need fixing too, part of this patch, or on its own.

I'm rather skeptical that it's useful to shrink the array at all.

It does seem that in the usual case, the memory saved in reallocation won't be as large as the memory used by the code to handle it. Plus that would obviate the need for fixing the realloc assumption.

[dim]

It's usually not worth the effort of shrinking, if the array doesn't get larger than a bunch of pages. How large is this one anyway, in a typical scenario? A dozen, or maybe two dozen entries?

[markj]

sys/kern/kern_osd.c

186

Right, (de)registration is rare (typically coincides with kldload and unload, I believe), and these arrays are not particularly big. In practice, it's quite possible that the reallocation won't save any memory at all, due to malloc(9)'s use of power-of-2-sized chunks of memory.