[PATCH] cifs: Fix the target file was deleted when rename failed.

[PATCH] cifs: Fix the target file was deleted when rename failed.

[Zhang Xiaoxu]

When xfstest generic/035, we found the target file was deleted
if the rename return -EACESS.

In cifs_rename2, we unlink the positive target dentry if rename
failed with EACESS or EEXIST, even if the target dentry is positived
before rename. Then the existing file was deleted.

We should just delete the target file which created during the
rename.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com>
Cc: stable@vger.kernel.org
---
 fs/cifs/inode.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ce95801e9b66..49c3ea8aa845 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -2044,6 +2044,7 @@ cifs_rename2(struct inode *source_dir, struct dentry *source_dentry,
 	FILE_UNIX_BASIC_INFO *info_buf_target;
 	unsigned int xid;
 	int rc, tmprc;
+	bool new_target = d_really_is_negative(target_dentry);
 
 	if (flags & ~RENAME_NOREPLACE)
 		return -EINVAL;
@@ -2120,8 +2121,13 @@ cifs_rename2(struct inode *source_dir, struct dentry *source_dentry,
 	 */
 
 unlink_target:
-	/* Try unlinking the target dentry if it's not negative */
-	if (d_really_is_positive(target_dentry) && (rc == -EACCES || rc == -EEXIST)) {
+	/*
+	 * If the target dentry was created during the rename, try
+	 * unlinking it if it's not negative
+	 */
+	if (new_target &&
+	    d_really_is_positive(target_dentry) &&
+	    (rc == -EACCES || rc == -EEXIST)) {
 		if (d_is_dir(target_dentry))
 			tmprc = cifs_rmdir(target_dir, target_dentry);
 		else
-- 
2.25.4

Re: [PATCH] cifs: Fix the target file was deleted when rename failed.

[Steve French]

tentatively merged into cifs-2.6.git pending review and testing

http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/5/builds/73

On Sun, Jun 28, 2020 at 8:05 PM Zhang Xiaoxu <zhangxiaoxu5@huawei.com> wrote:
>
> When xfstest generic/035, we found the target file was deleted
> if the rename return -EACESS.
>
> In cifs_rename2, we unlink the positive target dentry if rename
> failed with EACESS or EEXIST, even if the target dentry is positived
> before rename. Then the existing file was deleted.
>
> We should just delete the target file which created during the
> rename.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/cifs/inode.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index ce95801e9b66..49c3ea8aa845 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -2044,6 +2044,7 @@ cifs_rename2(struct inode *source_dir, struct dentry *source_dentry,
>         FILE_UNIX_BASIC_INFO *info_buf_target;
>         unsigned int xid;
>         int rc, tmprc;
> +       bool new_target = d_really_is_negative(target_dentry);
>
>         if (flags & ~RENAME_NOREPLACE)
>                 return -EINVAL;
> @@ -2120,8 +2121,13 @@ cifs_rename2(struct inode *source_dir, struct dentry *source_dentry,
>          */
>
>  unlink_target:
> -       /* Try unlinking the target dentry if it's not negative */
> -       if (d_really_is_positive(target_dentry) && (rc == -EACCES || rc == -EEXIST)) {
> +       /*
> +        * If the target dentry was created during the rename, try
> +        * unlinking it if it's not negative
> +        */
> +       if (new_target &&
> +           d_really_is_positive(target_dentry) &&
> +           (rc == -EACCES || rc == -EEXIST)) {
>                 if (d_is_dir(target_dentry))
>                         tmprc = cifs_rmdir(target_dir, target_dentry);
>                 else
> --
> 2.25.4
>


-- 
Thanks,

Steve

Re: [PATCH] cifs: Fix the target file was deleted when rename failed.

[Aurélien Aptel]

Reviewed-by: Aurelien Aptel <aaptel@suse.com>

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)