From 53ce9a3364de0723b27d861de93bfc882f7db050 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Wed, 17 Jul 2013 14:53:53 +0200 Subject: [PATCH 1/5] fuse: readdirplus: fix dentry leak In case d_lookup() returns a dentry with d_inode == NULL, the dentry is not returned with dput(). This results in triggering a BUG() in shrink_dcache_for_umount_subtree(): BUG: Dentry ...{i=0,n=...} still in use (1) [unmount of fuse fuse] [SzM: need to d_drop() as well] Reported-by: Justin Clift Signed-off-by: Niels de Vos Signed-off-by: Miklos Szeredi Tested-by: Brian Foster Tested-by: Niels de Vos CC: stable@vger.kernel.org --- fs/fuse/dir.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 0eda527..2ae5308 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1227,9 +1227,15 @@ static int fuse_direntplus_link(struct file *file, name.hash = full_name_hash(name.name, name.len); dentry = d_lookup(parent, &name); - if (dentry && dentry->d_inode) { + if (dentry) { inode = dentry->d_inode; - if (get_node_id(inode) == o->nodeid) { + if (!inode) { + d_drop(dentry); + } else if (get_node_id(inode) != o->nodeid) { + err = d_invalidate(dentry); + if (err) + goto out; + } else { struct fuse_inode *fi; fi = get_fuse_inode(inode); spin_lock(&fc->lock); @@ -1242,9 +1248,6 @@ static int fuse_direntplus_link(struct file *file, */ goto found; } - err = d_invalidate(dentry); - if (err) - goto out; dput(dentry); dentry = NULL; } -- 1.7.1 From a28ef45cbb1e7fadd5159deb17b02de15c6e4aaf Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Wed, 17 Jul 2013 14:53:53 +0200 Subject: [PATCH 2/5] fuse: readdirplus: sanity checks Add sanity checks before adding or updating an entry with data received from readdirplus. Signed-off-by: Miklos Szeredi CC: stable@vger.kernel.org --- fs/fuse/dir.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 2ae5308..bb68297 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1223,6 +1223,12 @@ static int fuse_direntplus_link(struct file *file, if (name.name[1] == '.' && name.len == 2) return 0; } + + if (invalid_nodeid(o->nodeid)) + return -EIO; + if (!fuse_valid_type(o->attr.mode)) + return -EIO; + fc = get_fuse_conn(dir); name.hash = full_name_hash(name.name, name.len); @@ -1231,10 +1237,14 @@ static int fuse_direntplus_link(struct file *file, inode = dentry->d_inode; if (!inode) { d_drop(dentry); - } else if (get_node_id(inode) != o->nodeid) { + } else if (get_node_id(inode) != o->nodeid || + ((o->attr.mode ^ inode->i_mode) & S_IFMT)) { err = d_invalidate(dentry); if (err) goto out; + } else if (is_bad_inode(inode)) { + err = -EIO; + goto out; } else { struct fuse_inode *fi; fi = get_fuse_inode(inode); -- 1.7.1 From 2914941e3178d84a216fc4eb85292dfef3b6d628 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Wed, 17 Jul 2013 14:53:53 +0200 Subject: [PATCH 3/5] fuse: readdirplus: fix instantiate Fuse does instantiation slightly differently from NFS/CIFS which use d_materialise_unique(). Signed-off-by: Miklos Szeredi CC: stable@vger.kernel.org --- fs/fuse/dir.c | 17 +++++++++++++---- 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index bb68297..5dfbb54 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1272,10 +1272,19 @@ static int fuse_direntplus_link(struct file *file, if (!inode) goto out; - alias = d_materialise_unique(dentry, inode); - err = PTR_ERR(alias); - if (IS_ERR(alias)) - goto out; + if (S_ISDIR(inode->i_mode)) { + mutex_lock(&fc->inst_mutex); + alias = fuse_d_add_directory(dentry, inode); + mutex_unlock(&fc->inst_mutex); + err = PTR_ERR(alias); + if (IS_ERR(alias)) { + iput(inode); + goto out; + } + } else { + alias = d_splice_alias(inode, dentry); + } + if (alias) { dput(dentry); dentry = alias; -- 1.7.1 From fa2b7213600f8110ebac64acebc78a885b0594a0 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Wed, 17 Jul 2013 14:53:53 +0200 Subject: [PATCH 4/5] fuse: readdirplus: change attributes once If we got the inode through fuse_iget() then the attributes are already up-to-date. Signed-off-by: Miklos Szeredi --- fs/fuse/dir.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 5dfbb54..37d85e0 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1252,6 +1252,10 @@ static int fuse_direntplus_link(struct file *file, fi->nlookup++; spin_unlock(&fc->lock); + fuse_change_attributes(inode, &o->attr, + entry_attr_timeout(o), + attr_version); + /* * The other branch to 'found' comes via fuse_iget() * which bumps nlookup inside @@ -1291,9 +1295,6 @@ static int fuse_direntplus_link(struct file *file, } found: - fuse_change_attributes(inode, &o->attr, entry_attr_timeout(o), - attr_version); - fuse_change_entry_timeout(dentry, o); err = 0; -- 1.7.1 From c7263bcdc4d150e3c718b711a5f6fad496d9f662 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Wed, 17 Jul 2013 14:53:54 +0200 Subject: [PATCH 5/5] fuse: readdirplus: cleanup Niels noted that we don't need the 'dentry = NULL' line. Signed-off-by: Miklos Szeredi CC: Niels de Vos --- fs/fuse/dir.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 37d85e0..72a5d5b 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1263,7 +1263,6 @@ static int fuse_direntplus_link(struct file *file, goto found; } dput(dentry); - dentry = NULL; } dentry = d_alloc(parent, &name); @@ -1299,8 +1298,7 @@ found: err = 0; out: - if (dentry) - dput(dentry); + dput(dentry); return err; } -- 1.7.1