vfs cache: Add vn_fullpath_jail(), factor out common code
ClosedPublic
Actions

Authored by olce on Sep 27 2025, 12:23 PM.

Details

Reviewers

mjg
markj
kib

Commits

rG2b5cc1e0e095: vfs cache: Add vn_fullpath_jail(), factor out common code
rG09ae06b1b224: vfs cache: Add vn_fullpath_jail(), factor out common code
rGc5a813c9f486: vfs cache: Add vn_fullpath_jail(), factor out common code

Summary

Introduce vn_fullpath_jail(), which returns a path to the passed vnode
relative to the current jail's root. It will be used by mac_do(4) in
a subsequent commit.

Factor out common code between the new variant and vn_fullpath(). While
here, rework the comments a bit.

MFC after: 3 days
Event: EuroBSDCon 2025
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 67380
Build 64263: arc lint + arc unit

Event Timeline

olce created this revision.Sep 27 2025, 12:23 PM

Herald added a subscriber: imp. · View Herald TranscriptSep 27 2025, 12:23 PM

olce requested review of this revision.Sep 27 2025, 12:23 PM

Harbormaster completed remote builds in B67362: Diff 162913.Sep 27 2025, 12:23 PM

olce added a child revision: D52758: MAC/do: Check executable path from the current jail's root.Sep 27 2025, 12:24 PM

olce edited the summary of this revision. (Show Details)Sep 27 2025, 12:31 PM

kib added inline comments.Sep 28 2025, 1:38 AM

sys/kern/vfs_cache.c
3383	I do not quite understand what do you mean by 'starts from the current chroot directory' there. Code actually means 'ends at the chroot'.
3400	Same there.

I've just discovered the vn_fullpath(9) manual page, which I should update with vn_fullpath_jail().

sys/kern/vfs_cache.c
3383	I'm just trying to describe what this function does from the users point of view. A path is generally described from the top, listing directories that have to be traversed to reach a file. Of course, the implementation actually starts from the target vnode and tries to climb the hierarchy. We could also describe that here. I've just noticed now that there is a `vn_fullpath(9)` manual page, which actually starts with a very similar description to the one introduced here.

Add vn_fullpath_jail() to vn_fullpath(9) and slightly tweak the latter

Herald added a subscriber: ziaee. · View Herald TranscriptSep 28 2025, 4:41 PM

Harbormaster completed remote builds in B67380: Diff 162998.Sep 28 2025, 4:41 PM

I don't have an opinion on the functionality, but the implementation looks avoidably slow.

This compiles to an indirect function call?

Given that the argument has to a vnode from pwd, you could instead pass an enum indicating which one is to be used. The compiler should be able to trivially convert that into an offset into the struct.

In D52757#1205724, @mjg wrote:

I don't have an opinion on the functionality, but the implementation looks avoidably slow.

In which way?

This compiles to an indirect function call?

It depends. Sometimes clang inlines the outer function and makes direct call to the parameter, effectively doing monomorphisation. For instance, this was the case for recent initgroups() patch.
Sometimes it does not.

Given that the argument has to a vnode from pwd, you could instead pass an enum indicating which one is to be used. The compiler should be able to trivially convert that into an offset into the struct.

THat said, when does vn_fullpath_anything() become a hot path requiring yet another uglification of the code in name of 'efficiency'?

There are more global vn_fullpath_XXX() functions, would be worth to enumerate all of them in the manpage.

sys/kern/vfs_cache.c
3345	You like to constify everything

This revision is now accepted and ready to land.Sep 29 2025, 3:19 AM

In D52757#1205724, @mjg wrote:

This compiles to an indirect function call?

LLVM has been smart enough for a relatively long time now about indirect function calls that are static, and it also inlines simple wrappers around not too long functions almost systematically. I've checked the produced assembly here, and that indirect function call has indeed been elided, as I have observed 100% out of several similar cases in the past years.

In D52757#1205782, @kib wrote:

There are more global vn_fullpath_XXX() functions, would be worth to enumerate all of them in the manpage.

Yeah, OK.

sys/kern/vfs_cache.c
3345	Ha ha, yes, it has become a second nature, as in admittedly rare occasions it has saved me some debugging time. It's one more way to indicate the invariants the person who wrote the code had in mind at that time, and sometimes it pays off.