[linux-cifs-client] Re: [PATCH] cifs: when renaming don't try to unlink negative dentry

[Steve French]

I merged this, adding the CC: stable, but think we need to also
consistently check for inode == NULL in cifs_unlink (we only check in
two branches now)

Any objections if I also add the following check:

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 09082ac..42572b7 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -968,7 +968,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
 	int xid;
 	char *full_path = NULL;
 	struct inode *inode = dentry->d_inode;
-	struct cifsInodeInfo *cifsInode = CIFS_I(inode);
+	struct cifsInodeInfo *cifs_inode;
 	struct super_block *sb = dir->i_sb;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
 	struct cifsTconInfo *tcon = cifs_sb->tcon;
@@ -1012,7 +1012,7 @@ psx_del_no_retry:
 		rc = cifs_rename_pending_delete(full_path, dentry, xid);
 		if (rc == 0)
 			drop_nlink(inode);
-	} else if (rc == -EACCES && dosattr == 0) {
+	} else if ((rc == -EACCES) && (dosattr == 0) && inode) {
 		attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
 		if (attrs == NULL) {
 			rc = -ENOMEM;
@@ -1020,7 +1020,8 @@ psx_del_no_retry:
 		}

 		/* try to reset dos attributes */
-		origattr = cifsInode->cifsAttrs;
+		cifs_inode = CIFS_I(inode);
+		origattr = cifs_inode->cifsAttrs;
 		if (origattr == 0)
 			origattr |= ATTR_NORMAL;
 		dosattr = origattr & ~ATTR_READONLY;
@@ -1041,13 +1042,13 @@ psx_del_no_retry:

 out_reval:
 	if (inode) {
-		cifsInode = CIFS_I(inode);
-		cifsInode->time = 0;	/* will force revalidate to get info
+		cifs_inode = CIFS_I(inode);
+		cifs_inode->time = 0;	/* will force revalidate to get info
 					   when needed */
 		inode->i_ctime = current_fs_time(sb);
 	}
 	dir->i_ctime = dir->i_mtime = current_fs_time(sb);
-	cifsInode = CIFS_I(dir);
+	cifs_inode = CIFS_I(dir);
 	CIFS_I(dir)->time = 0;	/* force revalidate of dir as well */

 	kfree(full_path);

On Fri, Apr 17, 2009 at 10:45 AM, Jeff Layton <jlayton at redhat.com> wrote:
> When attempting to rename a file on a read-only share, the kernel can
> call cifs_unlink on a negative dentry, which causes an oops. Only try
> to unlink the file if it's a positive dentry.
>
> Signed-off-by: Jeff Layton <jlayton at redhat.com>
> Tested-by: Shirish Pargaonkar <shirishp at us.ibm.com>
> ---
>  fs/cifs/inode.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 09082ac..f36b4e4 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1453,7 +1453,8 @@ int cifs_rename(struct inode *source_dir, struct dentry *source_dentry,
>                     checking the UniqueId via FILE_INTERNAL_INFO */
>
>  unlink_target:
> -       if ((rc == -EACCES) || (rc == -EEXIST)) {
> +       /* Try unlinking the target dentry if it's not negative */
> +       if (target_dentry->d_inode && (rc == -EACCES || rc == -EEXIST)) {
>                tmprc = cifs_unlink(target_dir, target_dentry);
>                if (tmprc)
>                        goto cifs_rename_exit;
> --
> 1.6.0.6
>
>



-- 
Thanks,

Steve