Re: [PATCH 7/9] net: centralize ns_common initialization
From: Jan Kara <jack@suse.cz>
Date: 2025-09-18 09:42:41
Also in:
linux-fsdevel, lkml, netdev
On Wed 17-09-25 12:28:06, Christian Brauner wrote:
quoted hunk ↗ jump to hunk
Centralize ns_common initialization. Signed-off-by: Christian Brauner <brauner@kernel.org> --- net/core/net_namespace.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index a57b3cda8dbc..897f4927df9e 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c@@ -409,7 +409,7 @@ static __net_init int preinit_net(struct net *net, struct user_namespace *user_n ns_ops = NULL; #endif - ret = ns_common_init(&net->ns, ns_ops, false); + ret = ns_common_init(&net->ns, ns_ops, true); if (ret) return ret;@@ -597,6 +597,7 @@ struct net *copy_net_ns(unsigned long flags, net_passive_dec(net); dec_ucounts: dec_net_namespaces(ucounts); + ns_free_inum(&net->ns);
This looks like a wrong place to put it? dec_ucounts also gets called when we failed to create 'net' and thus net == NULL.
quoted hunk ↗ jump to hunk
return ERR_PTR(rv); } return net;@@ -718,6 +719,7 @@ static void cleanup_net(struct work_struct *work) #endif put_user_ns(net->user_ns); net_passive_dec(net); + ns_free_inum(&net->ns);
The calling of ns_free_inum() after we've dropped our reference (net_passive_dec()) looks suspicious. Given how 'net' freeing works I don't think this can lead to actual UAF issues but it is in my opinion a bad coding pattern and for no good reason AFAICT.
} WRITE_ONCE(cleanup_net_task, NULL); }
Honza -- Jan Kara [off-list ref] SUSE Labs, CR