linux-kernel - RE: [PATCH] nsfs: fix oops when ns->ops is not provided

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <42fa84fe3dc148dea63096db24a039ae@AcuMS.aculab.com>
Date:   Wed, 2 Jun 2021 10:41:35 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Christian Brauner' <christian.brauner@...ntu.com>,
        Cong Wang <xiyou.wangcong@...il.com>
CC:     Jakub Kicinski <kuba@...nel.org>,
        Changbin Du <changbin.du@...il.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: RE: [PATCH] nsfs: fix oops when ns->ops is not provided

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)