From e401452d923de5b27f61f707773ec38f5593d985 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 18 Jun 2013 09:10:29 -0400 Subject: rpc_pipefs: only set rpc_dentry_ops if d_op isn't already set We had a report of a reproducible WARNING: [ 1360.039358] ------------[ cut here ]------------ [ 1360.043978] WARNING: at fs/dcache.c:1355 d_set_d_op+0x8d/0xc0() [ 1360.049880] Hardware name: HP Z200 Workstation [ 1360.054308] Modules linked in: nfsv4 nfs dns_resolver fscache nfsd auth_rpcgss nfs_acl lockd sunrpc sg acpi_cpufreq mperf coretemp kvm_intel kvm snd_hda_codec_realtek snd_hda_intel snd_hda_codec hp_wmi crc32c_intel snd_hwdep e1000e snd_seq snd_seq_device snd_pcm snd_page_alloc snd_timer snd sparse_keymap rfkill soundcore serio_raw ptp iTCO_wdt pps_core pcspkr iTCO_vendor_support mei microcode lpc_ich mfd_core wmi xfs libcrc32c sr_mod sd_mod cdrom crc_t10dif radeon i2c_algo_bit drm_kms_helper ttm ahci libahci drm i2c_core libata dm_mirror dm_region_hash dm_log dm_mod [last unloaded: auth_rpcgss] [ 1360.107406] Pid: 8814, comm: mount.nfs4 Tainted: G I -------------- 3.9.0-0.55.el7.x86_64 #1 [ 1360.116771] Call Trace: [ 1360.119219] [] warn_slowpath_common+0x70/0xa0 [ 1360.125208] [] warn_slowpath_null+0x1a/0x20 [ 1360.131025] [] d_set_d_op+0x8d/0xc0 [ 1360.136159] [] __rpc_lookup_create_exclusive+0x4f/0x80 [sunrpc] [ 1360.143710] [] rpc_mkpipe_dentry+0x86/0x170 [sunrpc] [ 1360.150311] [] nfs_idmap_new+0x96/0x130 [nfsv4] [ 1360.156475] [] nfs4_init_client+0xad/0x2d0 [nfsv4] [ 1360.162902] [] ? idr_get_empty_slot+0x16f/0x3c0 [ 1360.169062] [] ? idr_mark_full+0x52/0x60 [ 1360.174615] [] ? idr_alloc+0x79/0xe0 [ 1360.179826] [] ? __rpc_init_priority_wait_queue+0x81/0xc0 [sunrpc] [ 1360.187635] [] ? rpc_init_wait_queue+0x13/0x20 [sunrpc] [ 1360.194493] [] nfs_get_client+0x27a/0x350 [nfs] [ 1360.200666] [] nfs4_set_client.isra.8+0x78/0x100 [nfsv4] [ 1360.207624] [] nfs4_create_server+0xf3/0x3a0 [nfsv4] [ 1360.214222] [] nfs4_remote_mount+0x2e/0x60 [nfsv4] [ 1360.220644] [] mount_fs+0x39/0x1b0 [ 1360.225691] [] ? __alloc_percpu+0x10/0x20 [ 1360.231348] [] vfs_kern_mount+0x5f/0xf0 [ 1360.236822] [] nfs_do_root_mount+0x86/0xc0 [nfsv4] [ 1360.243246] [] nfs4_try_mount+0x44/0xc0 [nfsv4] [ 1360.249410] [] ? get_nfs_version+0x27/0x80 [nfs] [ 1360.255659] [] nfs_fs_mount+0x5c5/0xd10 [nfs] [ 1360.261650] [] ? nfs_clone_super+0x140/0x140 [nfs] [ 1360.268074] [] ? param_set_portnr+0x60/0x60 [nfs] [ 1360.274406] [] mount_fs+0x39/0x1b0 [ 1360.279443] [] ? __alloc_percpu+0x10/0x20 [ 1360.285088] [] vfs_kern_mount+0x5f/0xf0 [ 1360.290556] [] do_mount+0x1fd/0xa00 [ 1360.295677] [] ? __get_free_pages+0xe/0x50 [ 1360.301405] [] ? copy_mount_options+0x36/0x170 [ 1360.307479] [] sys_mount+0x83/0xc0 [ 1360.312515] [] system_call_fastpath+0x16/0x1b [ 1360.318503] ---[ end trace 8fa1f4cbc36094a7 ]--- The problem is that we're ending up in __rpc_lookup_create_exclusive with a negative dentry that already has d_op set. A little debugging has shown that when we hit this, the d_ops are already set to simple_dentry_operations. I believe that what's happening is that during a mount, idmapd is racing in and doing a lookup of /var/lib/nfs/rpc_pipefs/nfs/clnt???/idmap. Before that dentry reference is released, the kernel races in to create that file and finds the new negative dentry, which already has the d_op set. This patch just avoids setting the d_op if it's already set. simple_dentry_operations and rpc_dentry_operations are functionally equivalent so it shouldn't matter which one it's set to. Signed-off-by: Jeff Layton Signed-off-by: Trond Myklebust --- net/sunrpc/rpc_pipe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/sunrpc/rpc_pipe.c') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index e7ce4b3eb0bd..a816b3a69059 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -667,7 +667,8 @@ static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent, return ERR_PTR(-ENOMEM); } if (dentry->d_inode == NULL) { - d_set_d_op(dentry, &rpc_dentry_operations); + if (!dentry->d_op) + d_set_d_op(dentry, &rpc_dentry_operations); return dentry; } dput(dentry); -- cgit v1.2.3 From 384816051ca9125cd54750e59c780c2a2655fa4f Mon Sep 17 00:00:00 2001 From: Stanislav Kinsbursky Date: Mon, 24 Jun 2013 11:52:38 +0400 Subject: SUNRPC: fix races on PipeFS MOUNT notifications Below are races, when RPC client can be created without PiepFS dentries CPU#0 CPU#1 ----------------------------- ----------------------------- rpc_new_client rpc_fill_super rpc_setup_pipedir mutex_lock(&sn->pipefs_sb_lock) rpc_get_sb_net == NULL (no per-net PipeFS superblock) sn->pipefs_sb = sb; notifier_call_chain(MOUNT) (client is not in the list) rpc_register_client (client without pipes dentries) To fix this patch: 1) makes PipeFS mount notification call with pipefs_sb_lock being held. 2) releases pipefs_sb_lock on new SUNRPC client creation only after registration. Signed-off-by: Stanislav Kinsbursky Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust --- net/sunrpc/rpc_pipe.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'net/sunrpc/rpc_pipe.c') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index a816b3a69059..e02823bdfe98 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -1127,6 +1127,7 @@ rpc_fill_super(struct super_block *sb, void *data, int silent) return -ENOMEM; dprintk("RPC: sending pipefs MOUNT notification for net %p%s\n", net, NET_NAME(net)); + mutex_lock(&sn->pipefs_sb_lock); sn->pipefs_sb = sb; err = blocking_notifier_call_chain(&rpc_pipefs_notifier_list, RPC_PIPEFS_MOUNT, @@ -1134,6 +1135,7 @@ rpc_fill_super(struct super_block *sb, void *data, int silent) if (err) goto err_depopulate; sb->s_fs_info = get_net(net); + mutex_unlock(&sn->pipefs_sb_lock); return 0; err_depopulate: @@ -1142,6 +1144,7 @@ err_depopulate: sb); sn->pipefs_sb = NULL; __rpc_depopulate(root, files, RPCAUTH_lockd, RPCAUTH_RootEOF); + mutex_unlock(&sn->pipefs_sb_lock); return err; } -- cgit v1.2.3 From adb6fa7ffe9031857ec14b8aab75c9ab65556cbc Mon Sep 17 00:00:00 2001 From: Stanislav Kinsbursky Date: Wed, 26 Jun 2013 10:15:14 +0400 Subject: SUNRPC: fix races on PipeFS UMOUNT notifications CPU#0 CPU#1 ----------------------------- ----------------------------- rpc_kill_sb sn->pipefs_sb = NULL rpc_release_client (UMOUNT_EVENT) rpc_free_auth rpc_pipefs_event rpc_get_client_for_event !atomic_inc_not_zero(cl_count) atomic_inc(cl_count) rpc_free_client rpc_clnt_remove_pipedir To fix this, this patch does the following: 1) Calls RPC_PIPEFS_UMOUNT notification with sn->pipefs_sb_lock being held. 2) Removes SUNRPC client from the list AFTER pipes destroying. 3) Doesn't hold RPC client on notification: if client in the list, then it can't be destroyed while sn->pipefs_sb_lock in hold by notification caller. Signed-off-by: Stanislav Kinsbursky Cc: stable@vger.kernel.org Signed-off-by: Trond Myklebust --- net/sunrpc/rpc_pipe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sunrpc/rpc_pipe.c') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index e02823bdfe98..4679df5a6d50 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -1166,12 +1166,12 @@ static void rpc_kill_sb(struct super_block *sb) goto out; } sn->pipefs_sb = NULL; - mutex_unlock(&sn->pipefs_sb_lock); dprintk("RPC: sending pipefs UMOUNT notification for net %p%s\n", net, NET_NAME(net)); blocking_notifier_call_chain(&rpc_pipefs_notifier_list, RPC_PIPEFS_UMOUNT, sb); + mutex_unlock(&sn->pipefs_sb_lock); put_net(net); out: kill_litter_super(sb); -- cgit v1.2.3 From 76fa66657900071016f2bae61de28f059f3f2abf Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 2 Jul 2013 13:00:52 -0400 Subject: rpc_pipe: set dentry operations at d_alloc time Currently the way these get set is a little convoluted. If the dentry is allocated via lookup from userland, then it gets set by simple_lookup. If it gets allocated when the kernel is populating the directory, then it gets set via __rpc_lookup_create_exclusive, which has to check whether they might already be set. Between both of these, this ensures that all dentries have their d_op pointer set. Instead of doing that, just have them set at d_alloc time by pointing sb->s_d_op at them. With that change, we no longer want the lookup op to set them, so we must move to using our own lookup routine. Signed-off-by: Jeff Layton Signed-off-by: Trond Myklebust --- net/sunrpc/rpc_pipe.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) (limited to 'net/sunrpc/rpc_pipe.c') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index 4679df5a6d50..c5f6812ca06a 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -480,6 +480,23 @@ static const struct dentry_operations rpc_dentry_operations = { .d_delete = rpc_delete_dentry, }; +/* + * Lookup the data. This is trivial - if the dentry didn't already + * exist, we know it is negative. + */ +static struct dentry * +rpc_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags) +{ + if (dentry->d_name.len > NAME_MAX) + return ERR_PTR(-ENAMETOOLONG); + d_add(dentry, NULL); + return NULL; +} + +const struct inode_operations rpc_dir_inode_operations = { + .lookup = rpc_lookup, +}; + static struct inode * rpc_get_inode(struct super_block *sb, umode_t mode) { @@ -492,7 +509,7 @@ rpc_get_inode(struct super_block *sb, umode_t mode) switch (mode & S_IFMT) { case S_IFDIR: inode->i_fop = &simple_dir_operations; - inode->i_op = &simple_dir_inode_operations; + inode->i_op = &rpc_dir_inode_operations; inc_nlink(inode); default: break; @@ -666,11 +683,8 @@ static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent, if (!dentry) return ERR_PTR(-ENOMEM); } - if (dentry->d_inode == NULL) { - if (!dentry->d_op) - d_set_d_op(dentry, &rpc_dentry_operations); + if (dentry->d_inode == NULL) return dentry; - } dput(dentry); return ERR_PTR(-EEXIST); } @@ -1117,6 +1131,7 @@ rpc_fill_super(struct super_block *sb, void *data, int silent) sb->s_blocksize_bits = PAGE_CACHE_SHIFT; sb->s_magic = RPCAUTH_GSSMAGIC; sb->s_op = &s_ops; + sb->s_d_op = &rpc_dentry_operations; sb->s_time_gran = 1; inode = rpc_get_inode(sb, S_IFDIR | S_IRUGO | S_IXUGO); -- cgit v1.2.3 From 4f8568cb5290295c384d5c1328c52790e33a8a0d Mon Sep 17 00:00:00 2001 From: Fengguang Wu Date: Wed, 10 Jul 2013 09:17:14 +0800 Subject: rpc_pipe: rpc_dir_inode_operations can be static Hi Jeff, FYI, there are new sparse warnings show up in tree: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git nfs-for-next head: 296afe1f58d55fd56ed85daaafafcfee39f59ece commit: 76fa66657900071016f2bae61de28f059f3f2abf [2/5] rpc_pipe: set dentry operations at d_alloc time >> net/sunrpc/rpc_pipe.c:496:31: sparse: symbol 'rpc_dir_inode_operations' was not declared. Should it be static? Please consider folding the attached diff :-) Signed-off-by: Fengguang Wu Signed-off-by: Trond Myklebust --- net/sunrpc/rpc_pipe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sunrpc/rpc_pipe.c') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index c5f6812ca06a..61239a2cb786 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -493,7 +493,7 @@ rpc_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags) return NULL; } -const struct inode_operations rpc_dir_inode_operations = { +static const struct inode_operations rpc_dir_inode_operations = { .lookup = rpc_lookup, }; -- cgit v1.2.3 From a95e691f9c4a6e24fdeab6d7feae6d5411fe8a69 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 14 Jul 2013 16:43:54 +0400 Subject: rpc_create_*_dir: don't bother with qstr just pass the name Signed-off-by: Al Viro --- net/sunrpc/rpc_pipe.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'net/sunrpc/rpc_pipe.c') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index e7ce4b3eb0bd..63364cb5d11a 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -770,15 +770,17 @@ out_bad: } static struct dentry *rpc_mkdir_populate(struct dentry *parent, - struct qstr *name, umode_t mode, void *private, + const char *name, umode_t mode, void *private, int (*populate)(struct dentry *, void *), void *args_populate) { struct dentry *dentry; + struct qstr q = QSTR_INIT(name, strlen(name)); struct inode *dir = parent->d_inode; int error; + q.hash = full_name_hash(q.name, q.len); mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT); - dentry = __rpc_lookup_create_exclusive(parent, name); + dentry = __rpc_lookup_create_exclusive(parent, &q); if (IS_ERR(dentry)) goto out; error = __rpc_mkdir(dir, dentry, mode, NULL, private); @@ -925,8 +927,8 @@ static void rpc_clntdir_depopulate(struct dentry *dentry) /** * rpc_create_client_dir - Create a new rpc_client directory in rpc_pipefs - * @dentry: dentry from the rpc_pipefs root to the new directory - * @name: &struct qstr for the name + * @dentry: the parent of new directory + * @name: the name of new directory * @rpc_client: rpc client to associate with this directory * * This creates a directory at the given @path associated with @@ -935,7 +937,7 @@ static void rpc_clntdir_depopulate(struct dentry *dentry) * later be created using rpc_mkpipe(). */ struct dentry *rpc_create_client_dir(struct dentry *dentry, - struct qstr *name, + const char *name, struct rpc_clnt *rpc_client) { return rpc_mkdir_populate(dentry, name, S_IRUGO | S_IXUGO, NULL, @@ -981,7 +983,7 @@ static void rpc_cachedir_depopulate(struct dentry *dentry) rpc_depopulate(dentry, cache_pipefs_files, 0, 3); } -struct dentry *rpc_create_cache_dir(struct dentry *parent, struct qstr *name, +struct dentry *rpc_create_cache_dir(struct dentry *parent, const char *name, umode_t umode, struct cache_detail *cd) { return rpc_mkdir_populate(parent, name, umode, NULL, -- cgit v1.2.3 From d3db90b0a448bfdf77ab3d887c9579fead656cc5 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 14 Jul 2013 17:09:57 +0400 Subject: __rpc_lookup_create_exclusive: pass string instead of qstr ... and use d_hash_and_lookup() instead of open-coding it, for fsck sake... Signed-off-by: Al Viro --- net/sunrpc/rpc_pipe.c | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) (limited to 'net/sunrpc/rpc_pipe.c') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index 63364cb5d11a..27e54d265705 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -656,13 +656,12 @@ static int __rpc_rmpipe(struct inode *dir, struct dentry *dentry) } static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent, - struct qstr *name) + const char *name) { - struct dentry *dentry; - - dentry = d_lookup(parent, name); + struct qstr q = QSTR_INIT(name, strlen(name)); + struct dentry *dentry = d_hash_and_lookup(parent, &q); if (!dentry) { - dentry = d_alloc(parent, name); + dentry = d_alloc(parent, &q); if (!dentry) return ERR_PTR(-ENOMEM); } @@ -689,8 +688,7 @@ static void __rpc_depopulate(struct dentry *parent, for (i = start; i < eof; i++) { name.name = files[i].name; name.len = strlen(files[i].name); - name.hash = full_name_hash(name.name, name.len); - dentry = d_lookup(parent, &name); + dentry = d_hash_and_lookup(parent, &name); if (dentry == NULL) continue; @@ -732,12 +730,7 @@ static int rpc_populate(struct dentry *parent, mutex_lock(&dir->i_mutex); for (i = start; i < eof; i++) { - struct qstr q; - - q.name = files[i].name; - q.len = strlen(files[i].name); - q.hash = full_name_hash(q.name, q.len); - dentry = __rpc_lookup_create_exclusive(parent, &q); + dentry = __rpc_lookup_create_exclusive(parent, files[i].name); err = PTR_ERR(dentry); if (IS_ERR(dentry)) goto out_bad; @@ -774,13 +767,11 @@ static struct dentry *rpc_mkdir_populate(struct dentry *parent, int (*populate)(struct dentry *, void *), void *args_populate) { struct dentry *dentry; - struct qstr q = QSTR_INIT(name, strlen(name)); struct inode *dir = parent->d_inode; int error; - q.hash = full_name_hash(q.name, q.len); mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT); - dentry = __rpc_lookup_create_exclusive(parent, &q); + dentry = __rpc_lookup_create_exclusive(parent, name); if (IS_ERR(dentry)) goto out; error = __rpc_mkdir(dir, dentry, mode, NULL, private); @@ -843,7 +834,6 @@ struct dentry *rpc_mkpipe_dentry(struct dentry *parent, const char *name, struct dentry *dentry; struct inode *dir = parent->d_inode; umode_t umode = S_IFIFO | S_IRUSR | S_IWUSR; - struct qstr q; int err; if (pipe->ops->upcall == NULL) @@ -851,12 +841,8 @@ struct dentry *rpc_mkpipe_dentry(struct dentry *parent, const char *name, if (pipe->ops->downcall == NULL) umode &= ~S_IWUGO; - q.name = name; - q.len = strlen(name); - q.hash = full_name_hash(q.name, q.len), - mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT); - dentry = __rpc_lookup_create_exclusive(parent, &q); + dentry = __rpc_lookup_create_exclusive(parent, name); if (IS_ERR(dentry)) goto out; err = __rpc_mkpipe_dentry(dir, dentry, umode, &rpc_pipe_fops, @@ -1063,9 +1049,7 @@ struct dentry *rpc_d_lookup_sb(const struct super_block *sb, const unsigned char *dir_name) { struct qstr dir = QSTR_INIT(dir_name, strlen(dir_name)); - - dir.hash = full_name_hash(dir.name, dir.len); - return d_lookup(sb->s_root, &dir); + return d_hash_and_lookup(sb->s_root, &dir); } EXPORT_SYMBOL_GPL(rpc_d_lookup_sb); -- cgit v1.2.3 From dae3794fd603b92dcbac2859fe0bc7fe129a5188 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 14 Jul 2013 17:55:39 +0400 Subject: sunrpc: now we can just set ->s_d_op Signed-off-by: Al Viro --- net/sunrpc/rpc_pipe.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'net/sunrpc/rpc_pipe.c') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index 27e54d265705..260fe72656a1 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -665,10 +665,8 @@ static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent, if (!dentry) return ERR_PTR(-ENOMEM); } - if (dentry->d_inode == NULL) { - d_set_d_op(dentry, &rpc_dentry_operations); + if (dentry->d_inode == NULL) return dentry; - } dput(dentry); return ERR_PTR(-EEXIST); } @@ -1102,6 +1100,7 @@ rpc_fill_super(struct super_block *sb, void *data, int silent) sb->s_blocksize_bits = PAGE_CACHE_SHIFT; sb->s_magic = RPCAUTH_GSSMAGIC; sb->s_op = &s_ops; + sb->s_d_op = &rpc_dentry_operations; sb->s_time_gran = 1; inode = rpc_get_inode(sb, S_IFDIR | S_IRUGO | S_IXUGO); -- cgit v1.2.3