[PATCH] exfat: clean up new entry on add entry failure

[PATCH] exfat: clean up new entry on add entry failure

[Yichong Chen]

exfat_add_entry() initializes a new directory entry set before writing
it with exfat_put_dentry_set().  If the write fails, mkdir/create returns
an error but a partially written entry may be left behind.

For non-zero-size directories, the failure also happens after a cluster
has been allocated for the new directory.  That cluster needs to be
released if the mkdir fails.

Clean up the new entry best-effort and free the newly allocated directory
cluster when one was allocated.

Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
 fs/exfat/namei.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 038bc6fed681..7bd745090f92 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -471,6 +471,7 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 	struct exfat_entry_set_cache es;
 	int clu_size = 0;
 	unsigned int start_clu = EXFAT_FREE_CLUSTER;
+	bool dir_allocated = false;
 
 	ret = exfat_resolve_path(inode, path, &uniname);
 	if (ret)
@@ -497,6 +498,7 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 		}
 		start_clu = clu.dir;
 		clu_size = sbi->cluster_size;
+		dir_allocated = true;
 	}
 
 	/* update the directory entry */
@@ -507,8 +509,16 @@ static int exfat_add_entry(struct inode *inode, const char *path,
 	exfat_init_ext_entry(&es, num_entries, &uniname, NULL, 0);
 
 	ret = exfat_put_dentry_set(&es, IS_DIRSYNC(inode));
-	if (ret)
+	if (ret) {
+		if (!exfat_get_dentry_set(&es, sb, &info->dir, dentry,
+					  ES_ALL_ENTRIES)) {
+			exfat_remove_entries(inode, &es, ES_IDX_FILE, false);
+			exfat_put_dentry_set(&es, false);
+		}
+		if (dir_allocated)
+			exfat_free_cluster(inode, &clu);
 		goto out;
+	}
 
 	info->entry = dentry;
 	info->flags = ALLOC_NO_FAT_CHAIN;
-- 
2.51.0

Re: [PATCH] exfat: clean up new entry on add entry failure

[Namjae Jeon]

> diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
> index 038bc6fed681..7bd745090f92 100644
> --- a/fs/exfat/namei.c
> +++ b/fs/exfat/namei.c
> @@ -471,6 +471,7 @@ static int exfat_add_entry(struct inode *inode, const char *path,
>         struct exfat_entry_set_cache es;
>         int clu_size = 0;
>         unsigned int start_clu = EXFAT_FREE_CLUSTER;
> +       bool dir_allocated = false;
>
>         ret = exfat_resolve_path(inode, path, &uniname);
>         if (ret)
> @@ -497,6 +498,7 @@ static int exfat_add_entry(struct inode *inode, const char *path,
>                 }
>                 start_clu = clu.dir;
>                 clu_size = sbi->cluster_size;
> +               dir_allocated = true;
>         }
>
>         /* update the directory entry */
> @@ -507,8 +509,16 @@ static int exfat_add_entry(struct inode *inode, const char *path,
>         exfat_init_ext_entry(&es, num_entries, &uniname, NULL, 0);
>
>         ret = exfat_put_dentry_set(&es, IS_DIRSYNC(inode));
> -       if (ret)
> +       if (ret) {
> +               if (!exfat_get_dentry_set(&es, sb, &info->dir, dentry,
> +                                         ES_ALL_ENTRIES)) {
> +                       exfat_remove_entries(inode, &es, ES_IDX_FILE, false);
> +                       exfat_put_dentry_set(&es, false);
> +               }
> +               if (dir_allocated)
> +                       exfat_free_cluster(inode, &clu);
This can free a cluster even when the entry cleanup fails. Since
exfat_put_dentry_set() may fail after partially writing the new entry,
the on-disk entry could still reference the cluster. Freeing it in
that case may allow it to be reused and cause duplicated cluster
corruption. Please free the cluster only after the entry deletion has
completed successfully.

  if (ret) {
          int cleanup_ret;

          cleanup_ret = exfat_get_dentry_set(&es, sb, &info->dir,
                                             dentry, ES_ALL_ENTRIES);
          if (!cleanup_ret) {
                  exfat_remove_entries(inode, &es, ES_IDX_FILE, false);
                  cleanup_ret = exfat_put_dentry_set(&es, IS_DIRSYNC(inode));
          }

          if (!cleanup_ret && dir_allocated)
                  exfat_free_cluster(inode, &clu);
          goto out;
  }