linux-kernel - RE: [PATCH v3] exfat: do not clear VolumeDirty in writeback

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 22 Mar 2022 05:51:41 +0000
From:   "Kohada.Tetsuhiro@...MitsubishiElectric.co.jp" 
        <Kohada.Tetsuhiro@...MitsubishiElectric.co.jp>
To:     "'Yuezhang.Mo@...y.com'" <Yuezhang.Mo@...y.com>
CC:     "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "sj1557.seo@...sung.com" <sj1557.seo@...sung.com>,
        'Namjae Jeon' <linkinjeon@...nel.org>,
        "'Wataru.Aoyama@...y.com'" <Wataru.Aoyama@...y.com>,
        "'Andy.Wu@...y.com'" <Andy.Wu@...y.com>
Subject: RE: [PATCH v3] exfat: do not clear VolumeDirty in writeback

> Before this commit, VolumeDirty will be cleared first in writeback if 
> 'dirsync' or 'sync' is not enabled. If the power is suddenly cut off 
> after cleaning VolumeDirty but other updates are not written, the exFAT filesystem will not be able to detect the power failure in the next mount.
> 
> And VolumeDirty will be set again but not cleared when updating the 
> parent directory. It means that BootSector will be written at least once in each write-back, which will shorten the life of the device.
> 
> Reviewed-by: Andy Wu <Andy.Wu@...y.com>
> Reviewed-by: Aoyama Wataru <wataru.aoyama@...y.com>
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@...y.com>
> ---
> 
> Changes for v2:
>   - Clear VolumeDirty until sync or umount is run
> 
> Changes for v3:
>   - Add REQ_FUA and REQ_PREFLUSH to guarantee strict write ordering
> 
>  fs/exfat/file.c  |  2 --
>  fs/exfat/namei.c |  5 -----
>  fs/exfat/super.c | 10 ++--------
>  3 files changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/exfat/file.c b/fs/exfat/file.c index 
> d890fd34bb2d..2f5130059236 100644
> --- a/fs/exfat/file.c
> +++ b/fs/exfat/file.c
> @@ -218,8 +218,6 @@ int __exfat_truncate(struct inode *inode, loff_t new_size)
>  	if (exfat_free_cluster(inode, &clu))
>  		return -EIO;
> 
> -	exfat_clear_volume_dirty(sb);
> -
>  	return 0;
>  }
> 
> diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index 
> af4eb39cc0c3..39c9bdd6b6aa 100644
> --- a/fs/exfat/namei.c
> +++ b/fs/exfat/namei.c
> @@ -554,7 +554,6 @@ static int exfat_create(struct user_namespace *mnt_userns, struct inode *dir,
>  	exfat_set_volume_dirty(sb);
>  	err = exfat_add_entry(dir, dentry->d_name.name, &cdir, TYPE_FILE,
>  		&info);
> -	exfat_clear_volume_dirty(sb);
>  	if (err)
>  		goto unlock;
> 
> @@ -812,7 +811,6 @@ static int exfat_unlink(struct inode *dir, struct 
> dentry *dentry)
> 
>  	/* This doesn't modify ei */
>  	ei->dir.dir = DIR_DELETED;
> -	exfat_clear_volume_dirty(sb);
> 
>  	inode_inc_iversion(dir);
>  	dir->i_mtime = dir->i_atime = current_time(dir); @@ -846,7 +844,6 @@ 
> static int exfat_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
>  	exfat_set_volume_dirty(sb);
>  	err = exfat_add_entry(dir, dentry->d_name.name, &cdir, TYPE_DIR,
>  		&info);
> -	exfat_clear_volume_dirty(sb);
>  	if (err)
>  		goto unlock;
> 
> @@ -976,7 +973,6 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
>  		goto unlock;
>  	}
>  	ei->dir.dir = DIR_DELETED;
> -	exfat_clear_volume_dirty(sb);
> 
>  	inode_inc_iversion(dir);
>  	dir->i_mtime = dir->i_atime = current_time(dir); @@ -1311,7 +1307,6 
> @@ static int __exfat_rename(struct inode *old_parent_inode,
>  		 */
>  		new_ei->dir.dir = DIR_DELETED;
>  	}
> -	exfat_clear_volume_dirty(sb);
>  out:
>  	return ret;
>  }
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c index 
> 8c9fb7dcec16..c1f7f7b7c4ab 100644
> --- a/fs/exfat/super.c
> +++ b/fs/exfat/super.c
> @@ -100,7 +100,6 @@ static int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flags)  {
>  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
>  	struct boot_sector *p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
> -	bool sync;
> 
>  	/* retain persistent-flags */
>  	new_flags |= sbi->vol_flags_persistent; @@ -119,16 +118,11 @@ static 
> int exfat_set_vol_flags(struct super_block *sb, unsigned short 
> new_flags)
> 
>  	p_boot->vol_flags = cpu_to_le16(new_flags);
> 
> -	if ((new_flags & VOLUME_DIRTY) && !buffer_dirty(sbi->boot_bh))
> -		sync = true;
> -	else
> -		sync = false;
> -
>  	set_buffer_uptodate(sbi->boot_bh);
>  	mark_buffer_dirty(sbi->boot_bh);
> 
> -	if (sync)
> -		sync_dirty_buffer(sbi->boot_bh);
> +	__sync_dirty_buffer(sbi->boot_bh, REQ_SYNC | REQ_FUA | 
> +REQ_PREFLUSH);
> +
>  	return 0;
>  }

Looks good.

BR.
T.Kohada