lkml.org 
[lkml]   [2013]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [fuse-devel] [PATCH] fuse: fix occasional dentry leak when readdirplus is used
On Tue, Jul 16, 2013 at 09:15:16AM -0400, Brian Foster wrote:

> I'm not sure why it would need to have a valid inode. A dentry with a
> NULL inode is valid, no?

It is valid, yes. It's called a "negative" dentry, which caches the information
that the file does not exist.

> I think the question is whether the above state (multiple, hashed dentries)
> can be valid for whatever reason. It certainly looks suspicious.

That state is not valid. So we certainly need to unhash the negative dentry
first, before hashing a new one. We could also reuse the old dentry, but that
is just an optimization.

Below patch fixes several issues with that function. Could you please give it a
go?

Thanks,
Miklos


diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 0eda527..a8208c5 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1223,28 +1223,45 @@ 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);
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 ||
+ ((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);
spin_lock(&fc->lock);
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
*/
goto found;
}
- err = d_invalidate(dentry);
- if (err)
- goto out;
dput(dentry);
dentry = NULL;
}
@@ -1259,25 +1276,30 @@ 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;
}

found:
- fuse_change_attributes(inode, &o->attr, entry_attr_timeout(o),
- attr_version);
-
fuse_change_entry_timeout(dentry, o);

err = 0;
out:
- if (dentry)
- dput(dentry);
+ dput(dentry);
return err;
}


\
 
 \ /
  Last update: 2013-07-16 19:22    [W:0.089 / U:8.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site