From dc137bf553dbb6855bd7efc34fedcd03102455f7 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 16 Jul 2011 23:37:20 -0400 Subject: cifs: build_path_from_dentry() race fix deal with d_move() races properly; rename_lock read-retry loop, rcu_read_lock() held while walking to root, d_lock held over subtraction from namelen and copying the component to stabilize ->d_name. Signed-off-by: Al Viro --- fs/cifs/dir.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'fs/cifs/dir.c') diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 81914df47ef1..fa8c21d913bc 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -55,6 +55,7 @@ build_path_from_dentry(struct dentry *direntry) char dirsep; struct cifs_sb_info *cifs_sb = CIFS_SB(direntry->d_sb); struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb); + unsigned seq; if (direntry == NULL) return NULL; /* not much we can do if dentry is freed and @@ -68,22 +69,29 @@ build_path_from_dentry(struct dentry *direntry) dfsplen = 0; cifs_bp_rename_retry: namelen = dfsplen; + seq = read_seqbegin(&rename_lock); + rcu_read_lock(); for (temp = direntry; !IS_ROOT(temp);) { namelen += (1 + temp->d_name.len); temp = temp->d_parent; if (temp == NULL) { cERROR(1, "corrupt dentry"); + rcu_read_unlock(); return NULL; } } + rcu_read_unlock(); full_path = kmalloc(namelen+1, GFP_KERNEL); if (full_path == NULL) return full_path; full_path[namelen] = 0; /* trailing null */ + rcu_read_lock(); for (temp = direntry; !IS_ROOT(temp);) { + spin_lock(&temp->d_lock); namelen -= 1 + temp->d_name.len; if (namelen < 0) { + spin_unlock(&temp->d_lock); break; } else { full_path[namelen] = dirsep; @@ -91,14 +99,17 @@ cifs_bp_rename_retry: temp->d_name.len); cFYI(0, "name: %s", full_path + namelen); } + spin_unlock(&temp->d_lock); temp = temp->d_parent; if (temp == NULL) { cERROR(1, "corrupt dentry"); + rcu_read_unlock(); kfree(full_path); return NULL; } } - if (namelen != dfsplen) { + rcu_read_unlock(); + if (namelen != dfsplen || read_seqretry(&rename_lock, seq)) { cERROR(1, "did not end path lookup where expected namelen is %d", namelen); /* presumably this is only possible if racing with a rename -- cgit From dd7dd556e45133ef13f2c4bddc0e0b1ac23bc0e4 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 25 Jun 2011 21:17:17 -0400 Subject: no need to check for LOOKUP_OPEN in ->create() instances ... it will be set in nd->flag for all cases with non-NULL nd (i.e. when called from do_last()). Signed-off-by: Al Viro --- fs/9p/vfs_inode.c | 4 ++-- fs/9p/vfs_inode_dotl.c | 2 +- fs/cifs/dir.c | 6 +++--- fs/fuse/dir.c | 2 +- fs/nfs/dir.c | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) (limited to 'fs/cifs/dir.c') diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index 47f71eb66b32..7f9976a866e9 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -633,7 +633,7 @@ v9fs_vfs_create(struct inode *dir, struct dentry *dentry, int mode, fid = NULL; v9ses = v9fs_inode2v9ses(dir); perm = unixmode2p9mode(v9ses, mode); - if (nd && nd->flags & LOOKUP_OPEN) + if (nd) flags = nd->intent.open.flags; else flags = O_RDWR; @@ -649,7 +649,7 @@ v9fs_vfs_create(struct inode *dir, struct dentry *dentry, int mode, v9fs_invalidate_inode_attr(dir); /* if we are opening a file, assign the open fid to the file */ - if (nd && nd->flags & LOOKUP_OPEN) { + if (nd) { v9inode = V9FS_I(dentry->d_inode); mutex_lock(&v9inode->v_mutex); if (v9ses->cache && !v9inode->writeback_fid && diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c index d148e69f21b5..32bbbe5aa689 100644 --- a/fs/9p/vfs_inode_dotl.c +++ b/fs/9p/vfs_inode_dotl.c @@ -173,7 +173,7 @@ v9fs_vfs_create_dotl(struct inode *dir, struct dentry *dentry, int omode, struct posix_acl *pacl = NULL, *dacl = NULL; v9ses = v9fs_inode2v9ses(dir); - if (nd && nd->flags & LOOKUP_OPEN) + if (nd) flags = nd->intent.open.flags; else { /* diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index fa8c21d913bc..8766149f6300 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -179,7 +179,7 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode, if (oplockEnabled) oplock = REQ_OPLOCK; - if (nd && (nd->flags & LOOKUP_OPEN)) + if (nd) oflags = nd->intent.open.file->f_flags; else oflags = O_RDONLY | O_CREAT; @@ -214,7 +214,7 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode, which should be rare for path not covered on files) */ } - if (nd && (nd->flags & LOOKUP_OPEN)) { + if (nd) { /* if the file is going to stay open, then we need to set the desired access properly */ desiredAccess = 0; @@ -328,7 +328,7 @@ cifs_create_set_dentry: else cFYI(1, "Create worked, get_inode_info failed rc = %d", rc); - if (newinode && nd && (nd->flags & LOOKUP_OPEN)) { + if (newinode && nd) { struct cifsFileInfo *pfile_info; struct file *filp; diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 47559dd33193..02063dde2728 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -576,7 +576,7 @@ static int fuse_mknod(struct inode *dir, struct dentry *entry, int mode, static int fuse_create(struct inode *dir, struct dentry *entry, int mode, struct nameidata *nd) { - if (nd && (nd->flags & LOOKUP_OPEN)) { + if (nd) { int err = fuse_create_open(dir, entry, mode, nd); if (err != -ENOSYS) return err; diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index b5f63a50fa7f..77ae95f15497 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1585,7 +1585,7 @@ static int nfs_open_create(struct inode *dir, struct dentry *dentry, int mode, attr.ia_mode = mode; attr.ia_valid = ATTR_MODE; - if (nd && (nd->flags & LOOKUP_OPEN) != 0) + if (nd) open_flags = nd->intent.open.flags; ctx = create_nfs_open_context(dentry, open_flags); @@ -1596,7 +1596,7 @@ static int nfs_open_create(struct inode *dir, struct dentry *dentry, int mode, error = NFS_PROTO(dir)->create(dir, dentry, &attr, open_flags, ctx); if (error != 0) goto out_put_ctx; - if (nd && (nd->flags & LOOKUP_OPEN) != 0) { + if (nd) { error = nfs_intent_set_file(nd, ctx); if (error < 0) goto out_err; @@ -1675,7 +1675,7 @@ static int nfs_create(struct inode *dir, struct dentry *dentry, int mode, attr.ia_mode = mode; attr.ia_valid = ATTR_MODE; - if (nd && (nd->flags & LOOKUP_OPEN) != 0) + if (nd) open_flags = nd->intent.open.flags; error = NFS_PROTO(dir)->create(dir, dentry, &attr, open_flags, NULL); -- cgit From 407938e79edcadba1b5a17cf928584d8a191a8d7 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 25 Jun 2011 21:37:18 -0400 Subject: LOOKUP_CREATE and LOOKUP_RENAME_TARGET can be set only on the last step Signed-off-by: Al Viro --- fs/cifs/dir.c | 6 ++---- fs/fat/namei_vfat.c | 6 ++---- fs/jfs/namei.c | 6 ++---- 3 files changed, 6 insertions(+), 12 deletions(-) (limited to 'fs/cifs/dir.c') diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 8766149f6300..251c2ca569d3 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -663,10 +663,8 @@ cifs_d_revalidate(struct dentry *direntry, struct nameidata *nd) * case sensitive name which is specified by user if this is * for creation. */ - if (!(nd->flags & (LOOKUP_CONTINUE | LOOKUP_PARENT))) { - if (nd->flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET)) - return 0; - } + if (nd->flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET)) + return 0; if (time_after(jiffies, direntry->d_time + HZ) || !lookupCacheEnabled) return 0; diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c index 20b4ea53fdc4..bb3f29c3557b 100644 --- a/fs/fat/namei_vfat.c +++ b/fs/fat/namei_vfat.c @@ -82,10 +82,8 @@ static int vfat_revalidate_ci(struct dentry *dentry, struct nameidata *nd) * case sensitive name which is specified by user if this is * for creation. */ - if (!(nd->flags & (LOOKUP_CONTINUE | LOOKUP_PARENT))) { - if (nd->flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET)) - return 0; - } + if (nd->flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET)) + return 0; return vfat_revalidate_shortname(dentry); } diff --git a/fs/jfs/namei.c b/fs/jfs/namei.c index eaaf2b511e89..7a2e8e5152fd 100644 --- a/fs/jfs/namei.c +++ b/fs/jfs/namei.c @@ -1624,10 +1624,8 @@ static int jfs_ci_revalidate(struct dentry *dentry, struct nameidata *nd) * case sensitive name which is specified by user if this is * for creation. */ - if (!(nd->flags & (LOOKUP_CONTINUE | LOOKUP_PARENT))) { - if (nd->flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET)) - return 0; - } + if (nd->flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET)) + return 0; return 1; } -- cgit From 4352780386139ff33d2203868b392e6535deff61 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 25 Jun 2011 21:45:21 -0400 Subject: cifs_lookup(): LOOKUP_OPEN is set only on the last component Signed-off-by: Al Viro --- fs/cifs/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/cifs/dir.c') diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 251c2ca569d3..14d602f178c2 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -568,7 +568,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, * reduction in network traffic in the other paths. */ if (pTcon->unix_ext) { - if (nd && !(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) && + if (nd && !(nd->flags & LOOKUP_DIRECTORY) && (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open && (nd->intent.open.file->f_flags & O_CREAT)) { rc = cifs_posix_open(full_path, &newInode, -- cgit From 3ca30d40a91fb9b9871e61d5dea2c1a895906a15 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky Date: Mon, 25 Jul 2011 17:59:10 +0400 Subject: CIFS: Fix oops while mounting with prefixpath commit fec11dd9a0109fe52fd631e5c510778d6cbff6cc caused a regression when we have already mounted //server/share/a and want to mount //server/share/a/b. The problem is that lookup_one_len calls __lookup_hash with nd pointer as NULL. Then __lookup_hash calls do_revalidate in the case when dentry exists and we end up with NULL pointer deference in cifs_d_revalidate: if (nd->flags & LOOKUP_RCU) return -ECHILD; Fix this by checking nd for NULL. Signed-off-by: Pavel Shilovsky Signed-off-by: Al Viro --- fs/cifs/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/cifs/dir.c') diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 14d602f178c2..499f27fc8576 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -641,7 +641,7 @@ lookup_out: static int cifs_d_revalidate(struct dentry *direntry, struct nameidata *nd) { - if (nd->flags & LOOKUP_RCU) + if (nd && (nd->flags & LOOKUP_RCU)) return -ECHILD; if (direntry->d_inode) { -- cgit From e010a5ef95b8b6a12b74b548578f7dcf93564347 Mon Sep 17 00:00:00 2001 From: Steve French Date: Mon, 25 Jul 2011 22:04:32 +0000 Subject: [CIFS] Redundant null check after dereference Reviewed-by: Shirish Pargaonkar Signed-off-by: Steve French --- fs/cifs/dir.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'fs/cifs/dir.c') diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index fa8c21d913bc..8e9d37d44e9d 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -57,11 +57,6 @@ build_path_from_dentry(struct dentry *direntry) struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb); unsigned seq; - if (direntry == NULL) - return NULL; /* not much we can do if dentry is freed and - we need to reopen the file after it was closed implicitly - when the server crashed */ - dirsep = CIFS_DIR_SEP(cifs_sb); if (tcon->Flags & SMB_SHARE_IS_IN_DFS) dfsplen = strnlen(tcon->treeName, MAX_TREE_SIZE + 1); -- cgit From f5bc1e755d23d022bf948904386337fc3e5e29a8 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky Date: Mon, 25 Jul 2011 17:59:10 +0400 Subject: CIFS: Fix oops while mounting with prefixpath commit fec11dd9a0109fe52fd631e5c510778d6cbff6cc caused a regression when we have already mounted //server/share/a and want to mount //server/share/a/b. The problem is that lookup_one_len calls __lookup_hash with nd pointer as NULL. Then __lookup_hash calls do_revalidate in the case when dentry exists and we end up with NULL pointer deference in cifs_d_revalidate: if (nd->flags & LOOKUP_RCU) return -ECHILD; Fix this by checking nd for NULL. Signed-off-by: Pavel Shilovsky Reviewed-by: Shirish Pargaonkar CC: Stable Signed-off-by: Steve French --- fs/cifs/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/cifs/dir.c') diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 8e9d37d44e9d..c1bd0306d6c6 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -636,7 +636,7 @@ lookup_out: static int cifs_d_revalidate(struct dentry *direntry, struct nameidata *nd) { - if (nd->flags & LOOKUP_RCU) + if (nd && (nd->flags & LOOKUP_RCU)) return -ECHILD; if (direntry->d_inode) { -- cgit