On Wed, Jul 30, 2025 at 09:20:46AM -0700, Linus Torvalds wrote:

On Sat, 26 Jul 2025 at 18:35, Jakub Kicinski [off-list ref] wrote:

quoted

Networking changes for 6.17.

So while merging this, there was a trivial conflict with commit
9b0240b3ccc3 ("netns: use stable inode number for initial mount ns")
from the vfs side (acked by networking people).

And the conflict wasn't hard to resolve, but while looking at it, I
got very unhappy with that conflicting commit from the vfs tree.

Christian - when the "use stable inode number" code triggers, it
bypasses ns_alloc_inum() entirely. Fine - except that function *also*
does that

        WRITE_ONCE(ns->stashed, NULL);

so now ns->stashed isn't initialized any more.

Now, that shouldn't matter here because this only triggers for
'init_net' that is a global data structure and thus initialized to all
zeroes anyway, but it makes me very unhappy about that pattern that
ends up being about allocating the pid, but also almost incidentally
initializing that 'stashed' entry.

I ended up re-organizing the net_ns_net_init() code a bit (because it
now does that debugfs setup on success, so the old "return 0" didn't
work), and I think the merge is fine, but I think this "don't call
ns_alloc_inum()" pattern is wrong.

IOW, I don't think this is a bug, but I think it's not great.

I think we should not be initializing ns->stashed in ns_alloc_inum().
The function name is already wrong for that purpose:

static inline int ns_alloc_inum(struct ns_common *ns)
{
	WRITE_ONCE(ns->stashed, NULL);
	return proc_alloc_inum(&ns->inum);
}

That was done a long time ago via atomic_long_set() and I just changed
it to WRITE_ONCE() when I reworked both nsfs and pidfs.

We let all callers initialize the fields of struct ns_common embedded in
their respective namespace types already. I see no reason to not just do
the same thing for ns->stashed and drop that implicit initialization
from ns_alloc_inum().

But aside from that I think my patch should have probably been:

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 1b6f3826dd0e..5c39fb544f93 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c

@@ -815,7 +815,6 @@ static __net_init int net_ns_net_init(struct net *net)
 #ifdef CONFIG_NET_NS
        net->ns.ops = &netns_operations;
 #endif
-       net->ns.inum = PROC_NET_INIT_INO;
        if (net != &init_net) {
                int ret = ns_alloc_inum(&net->ns);
                if (ret)

@@ -1283,6 +1282,8 @@ void __init net_ns_init(void)
        init_net.key_domain = &init_net_key_domain;
 #endif
        preinit_net(&init_net, &init_user_ns);
+       init_net.ns.inum = PROC_NET_INIT_INO;
+       init_net.ns.stashed = NULL;

        down_write(&pernet_ops_rwsem);
        if (setup_net(&init_net))

so the setup for the initial network namespce happens right where it is
explicitly initialized.