On Tue, Jun 01, 2021 at 01:26:02PM -0700, Jakub Kicinski wrote:
On Tue, 1 Jun 2021 10:06:54 +0200 Christian Brauner wrote:
I'm not sure why we'd pick runtime checks for something that can be perfectly easily solved at compilation time. Networking should not be asking for FDs for objects which don't exist.
Agreed! This should be fixable by sm like:
diff --git a/net/socket.c b/net/socket.c index 27e3e7d53f8e..2484466d96ad 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1150,10 +1150,12 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg) break; case SIOCGSKNS: err = -EPERM; +#ifdef CONFIG_NET_NS if (!ns_capable(net->user_ns, CAP_NET_ADMIN)) break;
err = open_related_ns(&net->ns, get_net_ns);
+#endif break; case SIOCGSTAMP_OLD: case SIOCGSTAMPNS_OLD:
Thanks! You weren't CCed on v1, so FWIW I was suggesting checking in get_net_ns(), to catch other callers:
diff --git a/net/socket.c b/net/socket.c index 27e3e7d53f8e..3b44f2700e0c 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1081,6 +1081,8 @@ static long sock_do_ioctl(struct net *net, struct socket *sock, struct ns_common *get_net_ns(struct ns_common *ns) {
if (!IS_ENABLED(CONFIG_NET_NS))
return ERR_PTR(-EOPNOTSUPP); return &get_net(container_of(ns, struct net, ns))->ns;
} EXPORT_SYMBOL_GPL(get_net_ns);
Yeah, that's better than my hack. :) Maybe this function should simply move over to net/core/net_namespace.c with the other netns getters, e.g. get_net_ns_by_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);
Christian