Sorry for so long delay on this. Here is the fixed version of the patch:
[PATCH] CIFS: Fix NT_STATUS_ACCESS_DENIED for mounts with prefixpath option
but I found out one use case where it doesn't work right:
1) mount //server/share/a/b/ test
2) mount //server/share/ test2
3) stat test2/a/b - return wrong inode number (autodisable server inode number happens)
In the same time if I add "ls test2" before step #3 it works! May be VFS can't
handle negative dentry with children in this case, but I am not sure.
Reorganize code and make it send qpath info request only for a full
path (//server/share/prefixpath) rather than request for every path
compoment. In this case we end up with negative dentries for all
components except full one and we will instantiate them later if
such a mount is requested.
Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
---
fs/cifs/cifsfs.c | 138 +++++++++++++++++++++++++++++++--------------------
fs/cifs/cifsfs.h | 3 +-
fs/cifs/cifsglob.h | 6 ++
fs/cifs/dir.c | 3 +-
fs/cifs/inode.c | 7 ++-
fs/cifs/readdir.c | 43 ++++++++++------
6 files changed, 124 insertions(+), 76 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 8f1fe32..a188be1 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -90,14 +90,13 @@ extern mempool_t *cifs_sm_req_poolp;
extern mempool_t *cifs_req_poolp;
extern mempool_t *cifs_mid_poolp;
-static int
+static void
cifs_read_super(struct super_block *sb)
{
- struct inode *inode;
- struct cifs_sb_info *cifs_sb;
- int rc = 0;
+ struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
- cifs_sb = CIFS_SB(sb);
+ /* BB should we make this contingent on mount parm? */
+ sb->s_flags |= MS_NODIRATIME | MS_NOATIME;
if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIXACL)
sb->s_flags |= MS_POSIXACL;
@@ -115,26 +114,6 @@ cifs_read_super(struct super_block *sb)
sb->s_bdi = &cifs_sb->bdi;
sb->s_blocksize = CIFS_MAX_MSGSIZE;
sb->s_blocksize_bits = 14; /* default 2**14 = CIFS_MAX_MSGSIZE */
- inode = cifs_root_iget(sb);
-
- if (IS_ERR(inode)) {
- rc = PTR_ERR(inode);
- inode = NULL;
- goto out_no_root;
- }
-
- sb->s_root = d_alloc_root(inode);
-
- if (!sb->s_root) {
- rc = -ENOMEM;
- goto out_no_root;
- }
-
- /* do that *after* d_alloc_root() - we want NULL ->d_op for root here */
- if (cifs_sb_master_tcon(cifs_sb)->nocase)
- sb->s_d_op = &cifs_ci_dentry_ops;
- else
- sb->s_d_op = &cifs_dentry_ops;
#ifdef CONFIG_CIFS_NFSD_EXPORT
if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
@@ -143,14 +122,7 @@ cifs_read_super(struct super_block *sb)
}
#endif /* CONFIG_CIFS_NFSD_EXPORT */
- return 0;
-
-out_no_root:
- cERROR(1, "cifs_read_super: get root inode failed");
- if (inode)
- iput(inode);
-
- return rc;
+ sb->s_flags |= MS_ACTIVE;
}
static void cifs_kill_sb(struct super_block *sb)
@@ -528,6 +500,17 @@ static const struct super_operations cifs_super_ops = {
#endif
};
+static struct dentry *
+cifs_alloc_root(struct super_block *sb)
+{
+ struct qstr q;
+
+ q.name = "/";
+ q.len = 1;
+ q.hash = full_name_hash(q.name, q.len);
+ return d_alloc_pseudo(sb, &q);
+}
+
/*
* Get root dentry from superblock according to prefix path mount option.
* Return dentry with refcount + 1 on success and NULL otherwise.
@@ -535,8 +518,10 @@ static const struct super_operations cifs_super_ops = {
static struct dentry *
cifs_get_root(struct smb_vol *vol, struct super_block *sb)
{
- struct dentry *dentry;
+ struct dentry *dentry, *found;
+ struct inode *inode;
struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+ struct qstr q;
char *full_path = NULL;
char *s, *p;
char sep;
@@ -548,20 +533,30 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
cFYI(1, "Get root dentry for %s", full_path);
+ if (!sb->s_root) {
+ sb->s_root = cifs_alloc_root(sb);
+ if (IS_ERR(sb->s_root)) {
+ dentry = ERR_PTR(-ENOMEM);
+ goto out;
+ }
+
+ /*
+ * do that *after* cifs_alloc_root() - we want NULL ->d_op for
+ * root here
+ */
+ if (cifs_sb_master_tcon(cifs_sb)->nocase)
+ sb->s_d_op = &cifs_ci_dentry_ops;
+ else
+ sb->s_d_op = &cifs_dentry_ops;
+ }
+
sep = CIFS_DIR_SEP(cifs_sb);
dentry = dget(sb->s_root);
p = s = full_path;
do {
- struct inode *dir = dentry->d_inode;
struct dentry *child;
- if (!dir) {
- dput(dentry);
- dentry = ERR_PTR(-ENOENT);
- break;
- }
-
/* skip separators */
while (*s == sep)
s++;
@@ -572,12 +567,55 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
while (*s && *s != sep)
s++;
- mutex_lock(&dir->i_mutex);
- child = lookup_one_len(p, dentry, s - p);
- mutex_unlock(&dir->i_mutex);
+ q.name = p;
+ q.len = s - p;
+ if (dentry->d_flags & DCACHE_OP_HASH)
+ dentry->d_op->d_hash(dentry, dentry->d_inode, &q);
+ else
+ q.hash = full_name_hash(q.name, q.len);
+
+ child = d_lookup(dentry, &q);
+ if (!child) {
+ child = d_alloc(dentry, &q);
+ if (IS_ERR(child)) {
+ dput(dentry);
+ dentry = ERR_CAST(child);
+ break;
+ }
+ d_rehash(child);
+ }
dput(dentry);
dentry = child;
} while (!IS_ERR(dentry));
+
+ if (IS_ERR(dentry))
+ goto out;
+
+ mutex_lock(&cifs_root_mutex);
+
+ if (dentry->d_parent->d_inode)
+ mutex_lock(&dentry->d_parent->d_inode->i_mutex);
+
+ if (!dentry->d_inode) {
+ inode = cifs_mntroot_iget(sb, full_path);
+ if (IS_ERR(inode)) {
+ dput(dentry);
+ dentry = ERR_CAST(inode);
+ goto out;
+ }
+
+ found = d_instantiate_unique(dentry, inode);
+ if (found) {
+ dput(dentry);
+ dentry = found;
+ }
+ }
+
+ if (dentry->d_parent->d_inode)
+ mutex_unlock(&dentry->d_parent->d_inode->i_mutex);
+
+ mutex_unlock(&cifs_root_mutex);
+out:
kfree(full_path);
return dentry;
}
@@ -644,16 +682,7 @@ cifs_do_mount(struct file_system_type *fs_type,
cifs_umount(cifs_sb);
} else {
sb->s_flags = flags;
- /* BB should we make this contingent on mount parm? */
- sb->s_flags |= MS_NODIRATIME | MS_NOATIME;
-
- rc = cifs_read_super(sb);
- if (rc) {
- root = ERR_PTR(rc);
- goto out_super;
- }
-
- sb->s_flags |= MS_ACTIVE;
+ cifs_read_super(sb);
}
root = cifs_get_root(volume_info, sb);
@@ -1112,6 +1141,7 @@ init_cifs(void)
spin_lock_init(&cifs_tcp_ses_lock);
spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock);
+ mutex_init(&cifs_root_mutex);
if (cifs_max_pending < 2) {
cifs_max_pending = 2;
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 30ff560..cf28150 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -43,7 +43,8 @@ extern const struct address_space_operations cifs_addr_ops_smallbuf;
/* Functions related to inodes */
extern const struct inode_operations cifs_dir_inode_ops;
-extern struct inode *cifs_root_iget(struct super_block *);
+extern struct inode *cifs_mntroot_iget(struct super_block *sb,
+ const char *full_path);
extern int cifs_create(struct inode *, struct dentry *, int,
struct nameidata *);
extern struct dentry *cifs_lookup(struct inode *, struct dentry *,
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 8238aa1..67f6852 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -958,6 +958,12 @@ GLOBAL_EXTERN spinlock_t cifs_tcp_ses_lock;
*/
GLOBAL_EXTERN spinlock_t cifs_file_list_lock;
+/*
+ * This lock protects root dentry in cifs_get_root from being instantiated
+ * twice.
+ */
+GLOBAL_EXTERN struct mutex cifs_root_mutex;
+
#ifdef CONFIG_CIFS_DNOTIFY_EXPERIMENTAL /* unused temporarily */
/* Outstanding dir notify requests */
GLOBAL_EXTERN struct list_head GlobalDnotifyReqList;
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index d7eeb9d..d6b3319 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -553,6 +553,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
if (direntry->d_inode != NULL) {
cFYI(1, "non-NULL inode in lookup");
+ newInode = direntry->d_inode;
} else {
cFYI(1, "NULL inode in lookup");
}
@@ -596,7 +597,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
rc = cifs_get_inode_info(&newInode, full_path, NULL,
parent_dir_inode->i_sb, xid, NULL);
- if ((rc == 0) && (newInode != NULL)) {
+ if ((rc == 0) && (newInode != NULL) && !direntry->d_inode) {
d_add(direntry, newInode);
if (posix_open) {
filp = lookup_instantiate_filp(nd, direntry,
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 2c50bd2..7b7fccd 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -878,7 +878,7 @@ retry_iget5_locked:
}
/* gets root inode */
-struct inode *cifs_root_iget(struct super_block *sb)
+struct inode *cifs_mntroot_iget(struct super_block *sb, const char *full_path)
{
int xid;
struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
@@ -888,9 +888,10 @@ struct inode *cifs_root_iget(struct super_block *sb)
xid = GetXid();
if (tcon->unix_ext)
- rc = cifs_get_inode_info_unix(&inode, "", sb, xid);
+ rc = cifs_get_inode_info_unix(&inode, full_path, sb, xid);
else
- rc = cifs_get_inode_info(&inode, "", NULL, sb, xid, NULL);
+ rc = cifs_get_inode_info(&inode, full_path, NULL, sb, xid,
+ NULL);
if (!inode) {
inode = ERR_PTR(rc);
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 5de03ec..b2f8851 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -65,10 +65,8 @@ static inline void dump_cifs_file_struct(struct file *file, char *label)
}
#endif /* DEBUG2 */
-/*
- * Find the dentry that matches "name". If there isn't one, create one. If it's
- * a negative dentry or the uniqueid changed, then drop it and recreate it.
- */
+
+/* Find the dentry that matches "name". If there isn't one, create one. */
static struct dentry *
cifs_readdir_lookup(struct dentry *parent, struct qstr *name,
struct cifs_fattr *fattr)
@@ -84,32 +82,38 @@ cifs_readdir_lookup(struct dentry *parent, struct qstr *name,
else
name->hash = full_name_hash(name->name, name->len);
+ mutex_lock(&cifs_root_mutex);
+
dentry = d_lookup(parent, name);
if (dentry) {
/* FIXME: check for inode number changes? */
if (dentry->d_inode != NULL)
- return dentry;
- d_drop(dentry);
- dput(dentry);
+ goto out;
+ } else {
+ dentry = d_alloc(parent, name);
+ if (dentry == NULL)
+ goto out;
+ d_rehash(dentry);
}
- dentry = d_alloc(parent, name);
- if (dentry == NULL)
- return NULL;
-
inode = cifs_iget(sb, fattr);
if (!inode) {
dput(dentry);
- return NULL;
+ dentry = NULL;
+ goto out;
}
- alias = d_materialise_unique(dentry, inode);
+ alias = d_instantiate_unique(dentry, inode);
if (alias != NULL) {
dput(dentry);
- if (IS_ERR(alias))
- return NULL;
+ if (IS_ERR(alias)) {
+ dentry = NULL;
+ goto out;
+ }
dentry = alias;
}
+out:
+ mutex_unlock(&cifs_root_mutex);
return dentry;
}
@@ -708,6 +712,7 @@ int cifs_readdir(struct file *file, void *direntry, filldir_t filldir)
char *tmp_buf = NULL;
char *end_of_smb;
unsigned int max_len;
+ ino_t ino;
xid = GetXid();
@@ -732,8 +737,12 @@ int cifs_readdir(struct file *file, void *direntry, filldir_t filldir)
}
file->f_pos++;
case 1:
- if (filldir(direntry, "..", 2, file->f_pos,
- parent_ino(file->f_path.dentry), DT_DIR) < 0) {
+ if (!file->f_path.dentry->d_parent->d_inode) {
+ cFYI(1, "parent dir is negative, filling as current");
+ ino = file->f_path.dentry->d_inode->i_ino;
+ } else
+ ino = parent_ino(file->f_path.dentry);
+ if (filldir(direntry, "..", 2, file->f_pos, ino, DT_DIR) < 0) {
cERROR(1, "Filldir for parent dir failed");
rc = -ENOMEM;
break;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
