Page MenuHomeFreeBSD

kern: remove the need to allocate in prison_add_vfs()
Needs ReviewPublic

Authored by kevans on Aug 20 2025, 2:55 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 11, 3:00 PM
Unknown Object (File)
Sat, Oct 11, 3:00 PM
Unknown Object (File)
Sat, Oct 11, 6:35 AM
Unknown Object (File)
Sun, Oct 5, 5:21 AM
Unknown Object (File)
Thu, Oct 2, 3:02 AM
Unknown Object (File)
Wed, Oct 1, 4:43 AM
Unknown Object (File)
Tue, Sep 30, 8:28 AM
Unknown Object (File)
Fri, Sep 26, 2:15 AM

Details

Reviewers
None
Group Reviewers
Jails
Summary

The size of our format string is known at compile time, and vfc_name has
a similarly fixed size. asprintf() can theoretically fail and leave us
with no description, but there's really no need: the vfs type name is
small enough that the resulting stack requirement is fairly small in the
grand scheme of things (we won't even exceed 64-bytes) and it's unlikely
to increase dramatically (if at all).

This technically leaves us oversized because both sizeof() results will
include a NUL terminator, but it seems better to err on the side of
caution.

Diff Detail

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

Event Timeline

prison_add_vfs then goes on to call prison_add_allow, which has its own asprintf call. So unless you fix that as well, you've bought very little. And prison_add_allow really ought to be fixed. It's just a matter of not relying on asprintf with its M_NOWAIT allocation. The context it's called in (and presumably would ever be called in) can handle waiting for allocation.

I wonder why asprintf() uses M_NOWAIT when strdup() uses M_WAITOK. Looking at the relatively few callers, I suspect we can perhaps just switch to M_WAITOK...

I wonder why asprintf() uses M_NOWAIT when strdup() uses M_WAITOK. Looking at the relatively few callers, I suspect we can perhaps just switch to M_WAITOK...

Spot-checking asprintf callers has yielded eight optimists who assume it allocates, and one pessimist who just panics on failure. Not that jail's current silent failure is exactly better. And the good news is most callers check the return code and do something non-panicky.

I worry about just "fixing" asprintf though, since there are enough callers that it's non-trivial to verify their lock contexts. It might be better for prison_add_allow to just do its own asprintf-like work, and allocate the buffers sleepably and sprintf onto them.

FWIW- I don't actually care much about this path failing. It had just occurred to me while verifying that I can slightly move the prison_add_vfs() call in D52037 that the description really has a fixed max size.

The unrelated prison_add_allow talk deserves its own revision, so I made D52105 for that.