On Mon, Jun 29, 2026, at 7:45 AM, Christian Brauner wrote:

But I guess the even simpler model would be to copy what I've been doing
for pidfs:

[...]

we then add fchroot() (overdue anyway) and then teach both fchdir() and
fchroot() to honor FD_NULLFS_ROOT. Then a process may shed its fs state
and move itself into nullfs. Restrict *chdir() and *chroot() for said
process via seccomp and it's locked in forever as well.

This looks good! It delivers most of what I want, and I do want to be
very clear that while I am responding to your comments on my patch
below, I would still be very pleased if we just did this, much more than
I am pleased with the status quo.

(And also, yes, good to create the long-overdue fchroot regardless of
what we do here.)

Nothing here requires you to NULL anything and I oppose this on code
sanity reasons alone. We shoud absolutely not start to stash any NULL
pointers in core kernel objects such as struct path that are used
everywhere.

Before we do the "pidfd style" nullfs route, I want to make one thing
clear about my patch: I was *not* trying to relax the invariant across
the board that (live) `struct path` should only contain non-null
pointers. Rather, I just want `struct fs_struct` to contain ("morally")
`Option<struct path>`. My use of the null pointer was merely me doing
the sort of ragged union packing that, for example, Rust does. I think
as a matter of A_B_I (emphasis on "binary"), this is fine, and not
going to cause Armageddon --- `struct path` is widely used, but `struct
fs_struct` is (as far as I can tell) not.

All that said, as a matter of A_P_I (emphasis on "program"), I do see
your point that it's too easy for someone to not read my comment, and
then `struct path` with null pointers starts leaking all over the place,
making a big mess. I think a simple enough fix is to just use another C
encoding, such as a union, for `Option<struct path>`.

For example:

    union optional_path {
        struct {
            void *p0, *p1; /* must be null */
        } __randomize_layout null_path;
        struct path path; /* both fields must be non-null */
    };

To continue saving space, or --- if relying on the overlap of
`null_path` and `path.mnt` is too sketchy --- making a bona fide tagged
union:

    struct optional_path {
        enum {
            OPTIONAL_PATH_ABSENT,
            OPTIONAL_PATH_PRESENT,
        } tag;
        union {
            struct {} null_path;
            struct path path;
        };
    };

And either way, there can be an inline function:

    const struct path * /* nullable */
    get_optional_path(const struct optional_path * /* non-nullable */);

taking a non-null pointer and returning a nullable pointer to help
consumers of `struct fs_struct` not screw up accessing `root` and `pwd`.

A third option is simply copying the definition of `struct path`, doing:

    /* Just like `struct path`, but instead of both fields always being
     * non-null, both fields can also both be null to indicate an absent
     * path. One field null, the other field non-null is still not
     * permitted, however.
     */
    struct optional_path {
            struct vfsmount *optional_mnt;
            struct dentry *optional_dentry;
    } __randomize_layout;

in which case `get_optional_path` works by value instead of by
reference, because in the `CONFIG_RANDSTRUCT`-case the field order may
not be the same.

Any of these 3 variations would make absolutely clear that the
invariants around `struct path` have not changed, and only `struct
fs_struct` is changed. Furthermore, the API breakage on `fs->pwd` and
`fs->root` will mechanically ensure that all consumers get caught and
fixed (with the fix being to use `get_optional_path` and check for the
null case).

I do like these versions better than my original, because I do agree
making a safer C API is worthwhile. And because of the API breakage
forcing a complete patch as discussed above, I think that if I make a v2
along these lines, the diff will either prove or refute my basic premise
that `pwd` and `root` in `struct fs_struct`, unlike `struct path`, are
not widely used, and so changing their definitions like this (from
`struct path` to `... optional_path`) is lightweight.

Thanks,

John