Page MenuHomeFreeBSD
Differential D53728

kern: Introduce RLIMIT_VMM
ClosedPublic
Actions

Authored by bnovkov on Nov 13 2025, 11:13 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Aug 4, 6:05 PM
Unknown Object (File)
Mon, Aug 3, 1:52 PM
Unknown Object (File)
Mon, Aug 3, 7:12 AM
Unknown Object (File)
Sun, Aug 2, 8:37 AM
Unknown Object (File)
Sun, Aug 2, 6:34 AM
Unknown Object (File)
Sun, Aug 2, 3:50 AM
Unknown Object (File)
Sun, Aug 2, 2:55 AM
Unknown Object (File)
Sun, Aug 2, 1:15 AM
View All Files
Subscribers
corvink
imp
olce

Details

Reviewers
markj
olce
corvink
Group Reviewers
bhyve
Commits
rG1092ec8b3375: kern: Introduce RLIMIT_VMM
Summary

This change introduces a new per-UID limit for controlling the
number of vmm instances, in anticipation of unprivileged bhyve.
This limit allows us to control the maximum amount of kernel memory allocated
by the vmm driver and prevent potential memory exhaustion attacks.
The initial value of the limit was chosen arbitrarily and may be
adjusted in the future.

Sponsored by: The FreeBSD Foundation
Sponsored by: Klara, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 68594
Build 65477: arc lint + arc unit

Event Timeline

bnovkov created this revision.Nov 13 2025, 11:13 AM
Herald added a reviewer: olce. · View Herald TranscriptNov 13 2025, 11:13 AM
Herald added subscribers: olce, imp. · View Herald Transcript
bnovkov requested review of this revision.Nov 13 2025, 11:13 AM
Harbormaster completed remote builds in B68594: Diff 166350.Nov 13 2025, 11:13 AM
bnovkov added a child revision: D53729: vmm: Add ability to destroy VMs on close.Nov 13 2025, 11:13 AM
corvink added a subscriber: corvink.Nov 14 2025, 6:54 AM
corvink added inline comments.
sys/dev/vmm/vmm_dev.c
991

Why changing this to thread?

1190

It would be great to explain in the commit message why we want to restrict the number of vmms and especially, why we're choosing this default value.

bnovkov updated this revision to Diff 166564.Nov 17 2025, 2:45 PM
bnovkov edited the summary of this revision. (Show Details)

Address @corvink 's comments.

bnovkov added inline comments.Nov 17 2025, 2:47 PM
sys/dev/vmm/vmm_dev.c
991

Whoops, that was a leftover from an unrelated previous attempt, thanks for catching it!

1190

I've expanded the commit message, but the limit itself is fairly arbitrary. I'm open to suggestions if you think there's a more reasonable value.

corvink accepted this revision.Nov 18 2025, 6:53 AM
corvink added inline comments.
sys/dev/vmm/vmm_dev.c
1190

I just prefer that we're adding a short reason why choosing this default limit. If it's just a random pick, it's fine but it's worth mentioning in the commit message that it's just a random pick. So, if someone ever wonders why we've chosen this limit he can check the commit message.

This revision is now accepted and ready to land.Nov 18 2025, 6:53 AM
markj added a comment.Nov 18 2025, 3:16 PM

usr.bin/procstat/procstat_rlimit.c should be updated too.

sys/dev/vmm/vmm_dev.c
20

This sorts after queue.h.

1190

I agree, there should be a comment.

Thinking a bit more, having a max of 1024 is a bit silly, it's possibly too low on large count-count machines. Maybe let's just have it be 4*mp_ncpus or 8*mp_ncpus?

Also this needs to be overridable with a tunable, like hw.vmm.maxcpu above.

sys/kern/kern_resource.c
1776
bnovkov updated this revision to Diff 166883.Nov 21 2025, 10:07 AM
bnovkov edited the summary of this revision. (Show Details)

Address @markj 's comments

This revision now requires review to proceed.Nov 21 2025, 10:07 AM
bnovkov marked 4 inline comments as done.Nov 21 2025, 10:09 AM
markj added inline comments.Nov 21 2025, 3:07 PM
sys/dev/vmm/vmm_dev.c
1012

You'd need to decrement the counter again in the error cases below. Better would be to postpone it.

bnovkov updated this revision to Diff 167033.Nov 24 2025, 4:34 PM

Defer incrementing the vmm counter.

markj added inline comments.Nov 24 2025, 5:04 PM
sys/dev/vmm/vmm_dev.c
1041

You need to destroy the cdev here too. vmmdev_destroy() won't do it for you.

bnovkov updated this revision to Diff 167036.Nov 24 2025, 5:18 PM
bnovkov marked an inline comment as done.
bnovkov added inline comments.
sys/dev/vmm/vmm_dev.c
1041

whoops, that was silly, fixed!

olce added a comment.Nov 24 2025, 5:49 PM

There are already inline comments asking for a comment on the current limit. If possible, I think it would be great also to explain the point of this resource limit. In particular, which resources a VMM consumes that is not already accounted into existing resource limits, such as for memory: RLIMIT_AS, RLIMIT_RSS, RLIMIT_STACK, or CPU: RLIMIT_CPU? I suspect some kernel memory? Are there hardware limitations to the number of VMs that can be created?

sys/dev/vmm/vmm_dev.c
906–907

Small in-passing question: Can sc->ucred really be NULL here? I'm asking to be able to verify that chgvmmcnt() calls are indeed paired correctly.

bnovkov updated this revision to Diff 167384.Dec 1 2025, 8:20 PM
bnovkov edited the summary of this revision. (Show Details)

Address @olce 's comments.

bnovkov marked an inline comment as done.Dec 1 2025, 8:27 PM
In D53728#1231626, @olce wrote:

There are already inline comments asking for a comment on the current limit. If possible, I think it would be great also to explain the point of this resource limit. In particular, which resources a VMM consumes that is not already accounted into existing resource limits, such as for memory: RLIMIT_AS, RLIMIT_RSS, RLIMIT_STACK, or CPU: RLIMIT_CPU? I suspect some kernel memory? Are there hardware limitations to the number of VMs that can be created?

Thank you for the suggestion, the limit covers kernel memory allocated by the vmm driver (both the MI and platform-specific parts) and is meant to prevent resource exhaustion.
I've reworded the commit message to include this point.

sys/dev/vmm/vmm_dev.c
906–907

hm, I don't think it can since it'll always get set in vm_create. The same goes for sc->vm as well.
Looking at the commit history, I am not sure why these check have been placed.
I've converted the sc->ucred check into an assertion and will do the same for sc->vm in a separate patch.

olce accepted this revision.Dec 2 2025, 8:56 AM
This revision is now accepted and ready to land.Dec 2 2025, 8:56 AM
Closed by commit rG1092ec8b3375: kern: Introduce RLIMIT_VMM (authored by bnovkov). · Explain WhyDec 17 2025, 2:09 PM
This revision was automatically updated to reflect the committed changes.
bnovkov marked an inline comment as done.
bnovkov added a commit: rG1092ec8b3375: kern: Introduce RLIMIT_VMM.

Revision Contents
Changeset List

PathSize
sys/
dev/
vmm/
25 lines
kern/
13 lines
sys/
4 lines
2 lines

Diff 166350

View Options

sys/dev/vmm/vmm_dev.c

Loading...
View Options

sys/kern/kern_resource.c

Loading...
View Options

sys/sys/resource.h

Loading...
View Options

sys/sys/resourcevar.h

Loading...