From 31434f496abb9f3410b10f541462fe58613dd3ad Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Tue, 8 Jul 2014 11:45:48 +0800 Subject: nfs: check hostname in nfs_get_client We reference cl_hostname in many places. Add a check to make sure it exists. Signed-off-by: Peng Tao Signed-off-by: Trond Myklebust --- fs/nfs/client.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'fs/nfs/client.c') diff --git a/fs/nfs/client.c b/fs/nfs/client.c index 1d09289c8f0e..b213ee8fb012 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -482,8 +482,13 @@ nfs_get_client(const struct nfs_client_initdata *cl_init, struct nfs_net *nn = net_generic(cl_init->net, nfs_net_id); const struct nfs_rpc_ops *rpc_ops = cl_init->nfs_mod->rpc_ops; + if (cl_init->hostname == NULL) { + WARN_ON(1); + return NULL; + } + dprintk("--> nfs_get_client(%s,v%u)\n", - cl_init->hostname ?: "", rpc_ops->version); + cl_init->hostname, rpc_ops->version); /* see if the client already exists */ do { @@ -510,7 +515,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init, } while (!IS_ERR(new)); dprintk("<-- nfs_get_client() Failed to find %s (%ld)\n", - cl_init->hostname ?: "", PTR_ERR(new)); + cl_init->hostname, PTR_ERR(new)); return new; } EXPORT_SYMBOL_GPL(nfs_get_client); -- cgit v1.2.3 From f11b2a1cfbf5dd783eb55cb470509d06e20d1c78 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sat, 21 Jun 2014 20:52:17 -0400 Subject: nfs4: copy acceptor name from context to nfs_client The current CB_COMPOUND handling code tries to compare the principal name of the request with the cl_hostname in the client. This is not guaranteed to ever work, particularly if the client happened to mount a CNAME of the server or a non-fqdn. Fix this by instead comparing the cr_principal string with the acceptor name that we get from gssd. In the event that gssd didn't send one down (i.e. it was too old), then we fall back to trying to use the cl_hostname as we do today. Signed-off-by: Jeff Layton Signed-off-by: Trond Myklebust --- fs/nfs/client.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/nfs/client.c') diff --git a/fs/nfs/client.c b/fs/nfs/client.c index b213ee8fb012..168aa0df2658 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -252,6 +252,7 @@ void nfs_free_client(struct nfs_client *clp) put_net(clp->cl_net); put_nfs_version(clp->cl_nfs_mod); kfree(clp->cl_hostname); + kfree(clp->cl_acceptor); kfree(clp); dprintk("<-- nfs_free_client()\n"); -- cgit v1.2.3 From 1f70ef96b176bdb3b75230ec68850d83736b387b Mon Sep 17 00:00:00 2001 From: Alexey Khoroshilov Date: Fri, 18 Jul 2014 03:11:45 +0400 Subject: NFS: add checks for returned value of try_module_get() There is a couple of places in client code where returned value of try_module_get() is ignored. As a result there is a small chance to premature unload module because of unbalanced refcounting. The patch adds error handling in that places. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov Signed-off-by: Trond Myklebust --- fs/nfs/client.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'fs/nfs/client.c') diff --git a/fs/nfs/client.c b/fs/nfs/client.c index 168aa0df2658..b7bfa2765370 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -110,8 +110,8 @@ struct nfs_subversion *get_nfs_version(unsigned int version) mutex_unlock(&nfs_version_mutex); } - if (!IS_ERR(nfs)) - try_module_get(nfs->owner); + if (!IS_ERR(nfs) && !try_module_get(nfs->owner)) + return ERR_PTR(-EAGAIN); return nfs; } @@ -158,7 +158,8 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init) goto error_0; clp->cl_nfs_mod = cl_init->nfs_mod; - try_module_get(clp->cl_nfs_mod->owner); + if (!try_module_get(clp->cl_nfs_mod->owner)) + goto error_dealloc; clp->rpc_ops = clp->cl_nfs_mod->rpc_ops; @@ -190,6 +191,7 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init) error_cleanup: put_nfs_version(clp->cl_nfs_mod); +error_dealloc: kfree(clp); error_0: return ERR_PTR(err); -- cgit v1.2.3 From 65b38851a17472d31fec9019fc3a55b0802dab88 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 31 Jul 2014 04:35:20 -0700 Subject: NFS: Fix /proc/fs/nfsfs/servers and /proc/fs/nfsfs/volumes The usage of pid_ns->child_reaper->nsproxy->net_ns in nfs_server_list_open and nfs_client_list_open is not safe. /proc for a pid namespace can remain mounted after the all of the process in that pid namespace have exited. There are also times before the initial process in a pid namespace has started or after the initial process in a pid namespace has exited where pid_ns->child_reaper can be NULL or stale. Making the idiom pid_ns->child_reaper->nsproxy a double whammy of problems. Luckily all that needs to happen is to move /proc/fs/nfsfs/servers and /proc/fs/nfsfs/volumes under /proc/net to /proc/net/nfsfs/servers and /proc/net/nfsfs/volumes and add a symlink from the original location, and to use seq_open_net as it has been designed. Cc: stable@vger.kernel.org Cc: Trond Myklebust Cc: Stanislav Kinsbursky Signed-off-by: "Eric W. Biederman" --- fs/nfs/client.c | 95 +++++++++++++++++++++++++++++++++------------------------ 1 file changed, 55 insertions(+), 40 deletions(-) (limited to 'fs/nfs/client.c') diff --git a/fs/nfs/client.c b/fs/nfs/client.c index 1d09289c8f0e..180d1ec9c32e 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -1205,7 +1205,7 @@ static const struct file_operations nfs_server_list_fops = { .open = nfs_server_list_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = seq_release_net, .owner = THIS_MODULE, }; @@ -1226,7 +1226,7 @@ static const struct file_operations nfs_volume_list_fops = { .open = nfs_volume_list_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = seq_release_net, .owner = THIS_MODULE, }; @@ -1236,19 +1236,8 @@ static const struct file_operations nfs_volume_list_fops = { */ static int nfs_server_list_open(struct inode *inode, struct file *file) { - struct seq_file *m; - int ret; - struct pid_namespace *pid_ns = file->f_dentry->d_sb->s_fs_info; - struct net *net = pid_ns->child_reaper->nsproxy->net_ns; - - ret = seq_open(file, &nfs_server_list_ops); - if (ret < 0) - return ret; - - m = file->private_data; - m->private = net; - - return 0; + return seq_open_net(inode, file, &nfs_server_list_ops, + sizeof(struct seq_net_private)); } /* @@ -1256,7 +1245,7 @@ static int nfs_server_list_open(struct inode *inode, struct file *file) */ static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos) { - struct nfs_net *nn = net_generic(m->private, nfs_net_id); + struct nfs_net *nn = net_generic(seq_file_net(m), nfs_net_id); /* lock the list against modification */ spin_lock(&nn->nfs_client_lock); @@ -1268,7 +1257,7 @@ static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos) */ static void *nfs_server_list_next(struct seq_file *p, void *v, loff_t *pos) { - struct nfs_net *nn = net_generic(p->private, nfs_net_id); + struct nfs_net *nn = net_generic(seq_file_net(p), nfs_net_id); return seq_list_next(v, &nn->nfs_client_list, pos); } @@ -1278,7 +1267,7 @@ static void *nfs_server_list_next(struct seq_file *p, void *v, loff_t *pos) */ static void nfs_server_list_stop(struct seq_file *p, void *v) { - struct nfs_net *nn = net_generic(p->private, nfs_net_id); + struct nfs_net *nn = net_generic(seq_file_net(p), nfs_net_id); spin_unlock(&nn->nfs_client_lock); } @@ -1289,7 +1278,7 @@ static void nfs_server_list_stop(struct seq_file *p, void *v) static int nfs_server_list_show(struct seq_file *m, void *v) { struct nfs_client *clp; - struct nfs_net *nn = net_generic(m->private, nfs_net_id); + struct nfs_net *nn = net_generic(seq_file_net(m), nfs_net_id); /* display header on line 1 */ if (v == &nn->nfs_client_list) { @@ -1321,19 +1310,8 @@ static int nfs_server_list_show(struct seq_file *m, void *v) */ static int nfs_volume_list_open(struct inode *inode, struct file *file) { - struct seq_file *m; - int ret; - struct pid_namespace *pid_ns = file->f_dentry->d_sb->s_fs_info; - struct net *net = pid_ns->child_reaper->nsproxy->net_ns; - - ret = seq_open(file, &nfs_volume_list_ops); - if (ret < 0) - return ret; - - m = file->private_data; - m->private = net; - - return 0; + return seq_open_net(inode, file, &nfs_server_list_ops, + sizeof(struct seq_net_private)); } /* @@ -1341,7 +1319,7 @@ static int nfs_volume_list_open(struct inode *inode, struct file *file) */ static void *nfs_volume_list_start(struct seq_file *m, loff_t *_pos) { - struct nfs_net *nn = net_generic(m->private, nfs_net_id); + struct nfs_net *nn = net_generic(seq_file_net(m), nfs_net_id); /* lock the list against modification */ spin_lock(&nn->nfs_client_lock); @@ -1353,7 +1331,7 @@ static void *nfs_volume_list_start(struct seq_file *m, loff_t *_pos) */ static void *nfs_volume_list_next(struct seq_file *p, void *v, loff_t *pos) { - struct nfs_net *nn = net_generic(p->private, nfs_net_id); + struct nfs_net *nn = net_generic(seq_file_net(p), nfs_net_id); return seq_list_next(v, &nn->nfs_volume_list, pos); } @@ -1363,7 +1341,7 @@ static void *nfs_volume_list_next(struct seq_file *p, void *v, loff_t *pos) */ static void nfs_volume_list_stop(struct seq_file *p, void *v) { - struct nfs_net *nn = net_generic(p->private, nfs_net_id); + struct nfs_net *nn = net_generic(seq_file_net(p), nfs_net_id); spin_unlock(&nn->nfs_client_lock); } @@ -1376,7 +1354,7 @@ static int nfs_volume_list_show(struct seq_file *m, void *v) struct nfs_server *server; struct nfs_client *clp; char dev[8], fsid[17]; - struct nfs_net *nn = net_generic(m->private, nfs_net_id); + struct nfs_net *nn = net_generic(seq_file_net(m), nfs_net_id); /* display header on line 1 */ if (v == &nn->nfs_volume_list) { @@ -1407,6 +1385,45 @@ static int nfs_volume_list_show(struct seq_file *m, void *v) return 0; } +int nfs_fs_proc_net_init(struct net *net) +{ + struct nfs_net *nn = net_generic(net, nfs_net_id); + struct proc_dir_entry *p; + + nn->proc_nfsfs = proc_net_mkdir(net, "nfsfs", net->proc_net); + if (!nn->proc_nfsfs) + goto error_0; + + /* a file of servers with which we're dealing */ + p = proc_create("servers", S_IFREG|S_IRUGO, + nn->proc_nfsfs, &nfs_server_list_fops); + if (!p) + goto error_1; + + /* a file of volumes that we have mounted */ + p = proc_create("volumes", S_IFREG|S_IRUGO, + nn->proc_nfsfs, &nfs_volume_list_fops); + if (!p) + goto error_2; + return 0; + +error_2: + remove_proc_entry("servers", nn->proc_nfsfs); +error_1: + remove_proc_entry("fs/nfsfs", NULL); +error_0: + return -ENOMEM; +} + +void nfs_fs_proc_net_exit(struct net *net) +{ + struct nfs_net *nn = net_generic(net, nfs_net_id); + + remove_proc_entry("volumes", nn->proc_nfsfs); + remove_proc_entry("servers", nn->proc_nfsfs); + remove_proc_entry("fs/nfsfs", NULL); +} + /* * initialise the /proc/fs/nfsfs/ directory */ @@ -1419,14 +1436,12 @@ int __init nfs_fs_proc_init(void) goto error_0; /* a file of servers with which we're dealing */ - p = proc_create("servers", S_IFREG|S_IRUGO, - proc_fs_nfs, &nfs_server_list_fops); + p = proc_symlink("servers", proc_fs_nfs, "../../net/nfsfs/servers"); if (!p) goto error_1; /* a file of volumes that we have mounted */ - p = proc_create("volumes", S_IFREG|S_IRUGO, - proc_fs_nfs, &nfs_volume_list_fops); + p = proc_symlink("volumes", proc_fs_nfs, "../../net/nfsfs/volumes"); if (!p) goto error_2; return 0; -- cgit v1.2.3 From 21e81002f9788a3af591416b6dec60d7b67f2fb2 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Mon, 8 Sep 2014 16:17:55 -0700 Subject: nfs: fix kernel warning when removing proc entry I saw the following kernel warning: [ 1852.321222] ------------[ cut here ]------------ [ 1852.326527] WARNING: CPU: 0 PID: 118 at fs/proc/generic.c:521 remove_proc_entry+0x154/0x16b() [ 1852.335630] remove_proc_entry: removing non-empty directory 'fs/nfsfs', leaking at least 'volumes' [ 1852.344084] CPU: 0 PID: 118 Comm: kworker/u8:2 Not tainted 3.16.0+ #540 [ 1852.350036] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 1852.354992] Workqueue: netns cleanup_net [ 1852.358701] 0000000000000000 ffff880116f2fbd0 ffffffff819c03e9 ffff880116f2fc18 [ 1852.366474] ffff880116f2fc08 ffffffff810744ee ffffffff811e0e6e ffff8800d4e96238 [ 1852.373507] ffffffff81dbe665 ffff8800d46a5948 0000000000000005 ffff880116f2fc68 [ 1852.380224] Call Trace: [ 1852.381976] [] dump_stack+0x4d/0x66 [ 1852.385495] [] warn_slowpath_common+0x7a/0x93 [ 1852.389869] [] ? remove_proc_entry+0x154/0x16b [ 1852.393987] [] warn_slowpath_fmt+0x4c/0x4e [ 1852.397999] [] remove_proc_entry+0x154/0x16b [ 1852.402034] [] nfs_fs_proc_net_exit+0x53/0x56 [ 1852.406136] [] nfs_net_exit+0x12/0x1d [ 1852.409774] [] ops_exit_list+0x44/0x55 [ 1852.413529] [] cleanup_net+0xee/0x182 [ 1852.417198] [] process_one_work+0x209/0x40d [ 1852.502320] [] ? process_one_work+0x162/0x40d [ 1852.587629] [] worker_thread+0x1f0/0x2c7 [ 1852.673291] [] ? process_scheduled_works+0x2f/0x2f [ 1852.759470] [] kthread+0xc9/0xd1 [ 1852.843099] [] ? finish_task_switch+0x3a/0xce [ 1852.926518] [] ? __kthread_parkme+0x61/0x61 [ 1853.008565] [] ret_from_fork+0x7c/0xb0 [ 1853.076477] [] ? __kthread_parkme+0x61/0x61 [ 1853.140653] ---[ end trace 69c4c6617f78e32d ]--- It looks wrong that we add "/proc/net/nfsfs" in nfs_fs_proc_net_init() while remove "/proc/fs/nfsfs" in nfs_fs_proc_net_exit(). Fixes: commit 65b38851a17 (NFS: Fix /proc/fs/nfsfs/servers and /proc/fs/nfsfs/volumes) Cc: Eric W. Biederman Cc: Trond Myklebust Cc: Dan Aloni Signed-off-by: Cong Wang [Trond: replace uses of remove_proc_entry() with remove_proc_subtree() as suggested by Al Viro] Cc: stable@vger.kernel.org # 3.4.x : 65b38851a17: NFS: Fix /proc/fs/nfsfs/servers Cc: stable@vger.kernel.org # 3.4.x Signed-off-by: Trond Myklebust --- fs/nfs/client.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'fs/nfs/client.c') diff --git a/fs/nfs/client.c b/fs/nfs/client.c index 1c5ff6d58385..6a4f3666e273 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -1412,24 +1412,18 @@ int nfs_fs_proc_net_init(struct net *net) p = proc_create("volumes", S_IFREG|S_IRUGO, nn->proc_nfsfs, &nfs_volume_list_fops); if (!p) - goto error_2; + goto error_1; return 0; -error_2: - remove_proc_entry("servers", nn->proc_nfsfs); error_1: - remove_proc_entry("fs/nfsfs", NULL); + remove_proc_subtree("nfsfs", net->proc_net); error_0: return -ENOMEM; } void nfs_fs_proc_net_exit(struct net *net) { - struct nfs_net *nn = net_generic(net, nfs_net_id); - - remove_proc_entry("volumes", nn->proc_nfsfs); - remove_proc_entry("servers", nn->proc_nfsfs); - remove_proc_entry("fs/nfsfs", NULL); + remove_proc_subtree("nfsfs", net->proc_net); } /* -- cgit v1.2.3