Page MenuHomeFreeBSD

vmm: Destroy associated VM objects when a jail is destroyed

Authored by on Jul 19 2021, 11:24 PM.
Referenced Files
Unknown Object (File)
Mon, Jun 17, 1:38 PM
Unknown Object (File)
May 22 2024, 1:58 PM
Unknown Object (File)
May 22 2024, 4:52 AM
Unknown Object (File)
May 21 2024, 7:09 PM
Unknown Object (File)
May 15 2024, 7:15 PM
Unknown Object (File)
Apr 30 2024, 10:41 PM
Unknown Object (File)
Apr 19 2024, 6:22 AM
Unknown Object (File)
Jan 24 2024, 1:44 AM


Group Reviewers

...and also when vmm is disabled for a jail.

This revision builds off

Diff Detail

rG FreeBSD src repository
Lint Skipped
Tests Skipped

Event Timeline

markj added inline comments.

I would suggest asserting that vmmdev_mtx is held here.


Pedantically, buflen should be a size_t.


This is safe because vmmdev_destroy(), which frees sc, is called asynchronously. If that were not the case, and vmm_destroy() freed sc directly, then this would be a use-after-free, since after vmm_destroy() returns we would continue traversal by following sc->link.

I would suggest coding this a bit more defensively by using SLIST_FOREACH_SAFE to avoid referencing sc after vmm_destroy() returns, in case the implementation of vmm_destroy() changes at some point.


You could get rid of vmm_disabled and just write

if (vfs_flagopt(opts, "allow.novmm", NULL, 0))

I tend to think that this file should be merged with the other test file you added in D31156, renaming it to plain or so.

Fixed minor issues and merged test case files.

I'm not thrilled at the redundant call to vfs_flagopt(), which shouldn't be necessary because kern_jail_set has already looked for allow.novmm and set the permission bit accordingly. But by the time vmmdev_prison_set() is called, the old value of the permission bit is forgotten. So you're left with

  • Check the parameter directly (the current approach)
  • Check the permission bit, and call vmmdev_prison_cleanup() if it's not set (even it was already clear before and thus will have nothing to clear)
  • Going through all the effort of keeping your own state just for that one bit

Since all this happens only when jail_set(2) is called, which is hardly ever, there's no performance issue, and I guess it just comes down to what makes the code more readable (definitely not the third choice).

Just thinking out loud - I imagine you've already considered the options yourself and settled on what looked best.

This revision is now accepted and ready to land.Jul 27 2021, 11:10 PM