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.
- There are 3 different places (tun has two more) that need the same
fix.
- 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); }
- *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)