From: Christian Brauner > Sent: 02 June 2021 10:15 ... > Hm, I think a compile time check is better than a runtime check > independent of performance benefits. > > > > > 2) There are 3 different places (tun has two more) that need the same > > fix. > > > > > > 3) init_net always exits, except it does not have an ops when > > CONFIG_NET_NS is disabled: > > Which is true for every namespace. > > > > > static __net_init int net_ns_net_init(struct net *net) > > { > > #ifdef CONFIG_NET_NS > > net->ns.ops = &netns_operations; > > #endif > > return ns_alloc_inum(&net->ns); > > } > > > > 4) *I think* other namespaces need this fix too, for instance > > init_ipc_ns: > > None of them should have paths to trigger ->ops. > > > > > struct ipc_namespace init_ipc_ns = { > > .ns.count = REFCOUNT_INIT(1), > > .user_ns = &init_user_ns, > > .ns.inum = PROC_IPC_INIT_INO, > > #ifdef CONFIG_IPC_NS > > .ns.ops = &ipcns_operations, > > #endif > > }; > > > > whose ns->ops is NULL too if disabled. > > But the point is that ns->ops should never be accessed when that > namespace type is disabled. Or in other words, the bug is that something > in netns makes use of namespace features when they are disabled. If we > handle ->ops being NULL we might be tapering over a real bug somewhere. > > Jakub's proposal in the other mail makes sense and falls in line with > how the rest of the netns getters are implemented. For example > get_net_ns_fd_fd(): > > #ifdef CONFIG_NET_NS > > [...] > > struct net *get_net_ns_by_fd(int fd) > { > struct file *file; > struct ns_common *ns; > struct net *net; > > file = proc_ns_fget(fd); > if (IS_ERR(file)) > return ERR_CAST(file); > > ns = get_proc_ns(file_inode(file)); > if (ns->ops == &netns_operations) > net = get_net(container_of(ns, struct net, ns)); > else > net = ERR_PTR(-EINVAL); > > fput(file); > return net; > } > > #else > struct net *get_net_ns_by_fd(int fd) > { > return ERR_PTR(-EINVAL); > } > #endif > EXPORT_SYMBOL_GPL(get_net_ns_by_fd); > > (It seems that "get_net_ns()" could also be moved into the same file as > get_net_ns_by_fd() btw.) The default implementation ought to be in the .h file. So it gets inlined by the compiler. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)