Linux userland API discussions
 help / color / mirror / Atom feed
From: "John Ericson" <mail@johnericson.me>
To: "Christian Brauner" <brauner@kernel.org>
Cc: "Li Chen" <me@linux.beauty>, "Cong Wang" <cwang@multikernel.io>,
	linux-arch <linux-arch@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-api <linux-api@vger.kernel.org>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Thomas Gleixner" <tglx@kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Borislav Petkov" <bp@alien8.de>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>, "Jan Kara" <jack@suse.cz>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Kees Cook" <kees@kernel.org>,
	"Sergei Zimmerman" <sergei@zimmerman.foo>,
	"Farid Zakaria" <farid.m.zakaria@gmail.com>
Subject: Re: [RFC] Null Namespaces
Date: Mon, 29 Jun 2026 22:50:38 -0400	[thread overview]
Message-ID: <eb390c52-eeb3-44b3-88e9-e65c52a26c05@app.fastmail.com> (raw)
In-Reply-To: <20260629-hauer-erhitzen-sobald-96d3dff68707@brauner>

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

  parent reply	other threads:[~2026-06-30  2:52 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-24 22:51 [RFC] Null Namespaces John Ericson
2026-06-24 23:06 ` Andy Lutomirski
2026-06-24 23:20   ` Andy Lutomirski
2026-06-24 23:53     ` John Ericson
2026-06-25  1:10       ` Al Viro
2026-06-25  3:41         ` John Ericson
2026-06-25 15:51           ` Andy Lutomirski
2026-06-25 18:21             ` John Ericson
2026-06-26  0:15           ` Al Viro
2026-06-26 16:26             ` John Ericson
2026-06-29 10:31             ` Christian Brauner
2026-06-24 23:12 ` Al Viro
2026-06-25 21:00   ` H. Peter Anvin
2026-06-25 21:50     ` John Ericson
2026-06-25 23:09       ` Andy Lutomirski
2026-06-26  8:27         ` David Laight
2026-06-26 17:23           ` John Ericson
2026-06-29 10:39       ` Christian Brauner
2026-07-01  9:49         ` Jori Koolstra
2026-07-02 21:28           ` H. Peter Anvin
2026-06-29 11:45 ` Christian Brauner
2026-06-29 21:06   ` Andy Lutomirski
2026-06-30  4:25     ` John Ericson
2026-07-02  9:34     ` Christian Brauner
2026-07-02 15:43       ` John Ericson
2026-07-03  8:59         ` Christian Brauner
2026-07-03 17:35       ` Directory capability brain dump (Re: [RFC] Null Namespaces) Andy Lutomirski
2026-07-06 14:52       ` [RFC] Null Namespaces Christian Brauner
2026-07-06 16:32         ` Jann Horn
2026-07-06 17:10           ` Andy Lutomirski
2026-07-06 17:04         ` Andy Lutomirski
2026-06-30  2:50   ` John Ericson [this message]
2026-06-30  7:14     ` Christian Brauner
2026-06-30 17:20       ` John Ericson
2026-06-30 17:41         ` Andy Lutomirski
2026-07-02  9:29           ` Christian Brauner
2026-06-30 18:05         ` H. Peter Anvin
2026-07-04 13:20 ` Li Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eb390c52-eeb3-44b3-88e9-e65c52a26c05@app.fastmail.com \
    --to=mail@johnericson.me \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=brauner@kernel.org \
    --cc=corbet@lwn.net \
    --cc=cwang@multikernel.io \
    --cc=dave.hansen@linux.intel.com \
    --cc=farid.m.zakaria@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jack@suse.cz \
    --cc=kees@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=me@linux.beauty \
    --cc=mingo@redhat.com \
    --cc=sergei@zimmerman.foo \
    --cc=skhan@linuxfoundation.org \
    --cc=tglx@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox