| 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
| ||
|
Date: Mon, 13 Jul 2020 13:52:43 +0900
From: "Namjae Jeon" <namjae.jeon@...sung.com>
To: "'Tetsuhiro Kohada'" <kohada.t2@...il.com>
Cc: <kohada.tetsuhiro@...mitsubishielectric.co.jp>,
<mori.takahiro@...mitsubishielectric.co.jp>,
<motai.hirotaka@...mitsubishielectric.co.jp>,
"'Sungjong Seo'" <sj1557.seo@...sung.com>,
<linux-fsdevel@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] exfat: retain 'VolumeFlags' properly
Hi Tetsuhiro,
> Retain ActiveFat, MediaFailure and ClearToZero fields.
> And, never clear VolumeDirty, if it is dirty at mount.
>
> In '3.1.13.3 Media Failure Field' of exfat specification says ...
> If, upon mounting a volume, the value of this field is 1, implementations which scan the entire
> volume for media failures and record all failures as "bad" clusters in the FAT (or otherwise resolve
> media failures) may clear the value of this field to 0.
> Therefore, should not clear MediaFailure without scanning volume.
>
> In '8.1 Recommended Write Ordering' of exfat specification says ...
> Clear the value of the VolumeDirty field to 0, if its value prior to the first step was 0 Therefore,
> should not clear VolumeDirty when mounted.
>
> Also, rename ERR_MEDIUM to MED_FAILURE.
I think that MEDIA_FAILURE looks better.
>
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@...il.com>
> ---
> fs/exfat/exfat_fs.h | 5 +++--
> fs/exfat/exfat_raw.h | 2 +-
> fs/exfat/super.c | 22 ++++++++++++++--------
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index cb51d6e83199..3f8dc4ca8109 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -224,7 +224,8 @@ struct exfat_sb_info {
> unsigned int num_FAT_sectors; /* num of FAT sectors */
> unsigned int root_dir; /* root dir cluster */
> unsigned int dentries_per_clu; /* num of dentries per cluster */
> - unsigned int vol_flag; /* volume dirty flag */
> + unsigned int vol_flags; /* volume flags */
> + unsigned int vol_flags_noclear; /* volume flags to retain */
> struct buffer_head *boot_bh; /* buffer_head of BOOT sector */
>
> unsigned int map_clu; /* allocation bitmap start cluster */ @@ -380,7 +381,7 @@ static inline
> int exfat_sector_to_cluster(struct exfat_sb_info *sbi, }
>
> /* super.c */
> -int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flag);
> +int exfat_set_vol_flags(struct super_block *sb, unsigned short
> +new_flags);
>
> /* fatent.c */
> #define exfat_get_next_cluster(sb, pclu) exfat_ent_get(sb, *(pclu), pclu) diff --git
> a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h index 350ce59cc324..d86a8a6b0601 100644
> --- a/fs/exfat/exfat_raw.h
> +++ b/fs/exfat/exfat_raw.h
> @@ -16,7 +16,7 @@
>
> #define VOL_CLEAN 0x0000
> #define VOL_DIRTY 0x0002
> -#define ERR_MEDIUM 0x0004
> +#define MED_FAILURE 0x0004
>
> #define EXFAT_EOF_CLUSTER 0xFFFFFFFFu
> #define EXFAT_BAD_CLUSTER 0xFFFFFFF7u
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c index b5bf6dedbe11..c26b0f5a0875 100644
> --- a/fs/exfat/super.c
> +++ b/fs/exfat/super.c
> @@ -96,17 +96,22 @@ static int exfat_statfs(struct dentry *dentry, struct kstatfs *buf)
> return 0;
> }
>
> -int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flag)
> +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;
If dirty bit is set in on-disk volume flags, we can just return 0 at the beginning
of this function ?
>
> + if (new_flags == VOL_CLEAN)
> + new_flags = (sbi->vol_flags & ~VOL_DIRTY) | sbi->vol_flags_noclear;
> + else
> + new_flags |= sbi->vol_flags;
> +
> /* flags are not changed */
> - if (sbi->vol_flag == new_flag)
> + if (sbi->vol_flags == new_flags)
> return 0;
>
> - sbi->vol_flag = new_flag;
> + sbi->vol_flags = new_flags;
>
> /* skip updating volume dirty flag,
> * if this volume has been mounted with read-only @@ -114,9 +119,9 @@ int
> exfat_set_vol_flags(struct super_block *sb, unsigned short new_flag)
> if (sb_rdonly(sb))
> return 0;
>
> - p_boot->vol_flags = cpu_to_le16(new_flag);
> + p_boot->vol_flags = cpu_to_le16(new_flags);
How about set or clear only dirty bit to on-disk volume flags instead of using
sbi->vol_flags_noclear ?
if (set)
p_boot->vol_flags |= cpu_to_le16(VOL_DIRTY);
else
p_boot->vol_flags &= cpu_to_le16(~VOL_DIRTY);
>
> - if (new_flag == VOL_DIRTY && !buffer_dirty(sbi->boot_bh))
> + if ((new_flags & VOL_DIRTY) && !buffer_dirty(sbi->boot_bh))
> sync = true;
> else
> sync = false;
> @@ -457,7 +462,8 @@ static int exfat_read_boot_sector(struct super_block *sb)
> sbi->dentries_per_clu = 1 <<
> (sbi->cluster_size_bits - DENTRY_SIZE_BITS);
>
> - sbi->vol_flag = le16_to_cpu(p_boot->vol_flags);
> + sbi->vol_flags = le16_to_cpu(p_boot->vol_flags);
> + sbi->vol_flags_noclear = sbi->vol_flags & (VOL_DIRTY | MED_FAILURE);
> sbi->clu_srch_ptr = EXFAT_FIRST_CLUSTER;
> sbi->used_clusters = EXFAT_CLUSTERS_UNTRACKED;
>
> @@ -472,9 +478,9 @@ static int exfat_read_boot_sector(struct super_block *sb)
> exfat_err(sb, "bogus data start sector");
> return -EINVAL;
> }
> - if (sbi->vol_flag & VOL_DIRTY)
> + if (sbi->vol_flags & VOL_DIRTY)
> exfat_warn(sb, "Volume was not properly unmounted. Some data may be corrupt. Please run
> fsck.");
> - if (sbi->vol_flag & ERR_MEDIUM)
> + if (sbi->vol_flags & MED_FAILURE)
> exfat_warn(sb, "Medium has reported failures. Some data may be lost.");
>
> /* exFAT file size is limited by a disk volume size */
> --
> 2.25.1
Powered by blists - more mailing lists