linux-kernel - Re: [PATCH v3] exfat: check disk status during buffer write

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAKYAXd9SbOfkHCeynzdhF-H4ejxSGoz1Bhv60qJmgQQkpQsptA@mail.gmail.com>
Date: Wed, 31 Jul 2024 16:26:19 +0900
From: Namjae Jeon <linkinjeon@...nel.org>
To: Dongliang Cui <dongliang.cui@...soc.com>
Cc: sj1557.seo@...sung.com, hch@...radead.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, niuzhiguo84@...il.com, hao_hao.wang@...soc.com, 
	ke.wang@...soc.com, Zhiguo Niu <zhiguo.niu@...soc.com>
Subject: Re: [PATCH v3] exfat: check disk status during buffer write

2024년 7월 31일 (수) 오전 11:29, Dongliang Cui <dongliang.cui@...soc.com>님이 작성:
>
> We found that when writing a large file through buffer write, if the
> disk is inaccessible, exFAT does not return an error normally, which
> leads to the writing process not stopping properly.
>
> To easily reproduce this issue, you can follow the steps below:
>
> 1. format a device to exFAT and then mount (with a full disk erase)
> 2. dd if=/dev/zero of=/exfat_mount/test.img bs=1M count=8192
> 3. eject the device
>
> You may find that the dd process does not stop immediately and may
> continue for a long time.
>
> The root cause of this issue is that during buffer write process,
> exFAT does not need to access the disk to look up directory entries
> or the FAT table (whereas FAT would do) every time data is written.
> Instead, exFAT simply marks the buffer as dirty and returns,
> delegating the writeback operation to the writeback process.
>
> If the disk cannot be accessed at this time, the error will only be
> returned to the writeback process, and the original process will not
> receive the error, so it cannot be returned to the user side.
>
> When the disk cannot be accessed normally, an error should be returned
> to stop the writing process.
>
> Signed-off-by: Dongliang Cui <dongliang.cui@...soc.com>
> Signed-off-by: Zhiguo Niu <zhiguo.niu@...soc.com>
> ---
> Changes in v3:
>  - Implement .shutdown to monitor disk status.
> ---
>  fs/exfat/exfat_fs.h | 10 ++++++++++
>  fs/exfat/inode.c    |  3 +++
>  fs/exfat/super.c    | 11 +++++++++++
>  3 files changed, 24 insertions(+)
>
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
> index ecc5db952deb..c6cf36070aa3 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -148,6 +148,9 @@ enum {
>  #define DIR_CACHE_SIZE         \
>         (DIV_ROUND_UP(EXFAT_DEN_TO_B(ES_MAX_ENTRY_NUM), SECTOR_SIZE) + 1)
>
> +/* Superblock flags */
> +#define EXFAT_FLAGS_SHUTDOWN   1
> +
>  struct exfat_dentry_namebuf {
>         char *lfn;
>         int lfnbuf_len; /* usually MAX_UNINAME_BUF_SIZE */
> @@ -267,6 +270,8 @@ struct exfat_sb_info {
>         unsigned int clu_srch_ptr; /* cluster search pointer */
>         unsigned int used_clusters; /* number of used clusters */
>
> +       unsigned long s_exfat_flags; /* Exfat superblock flags */
> +
>         struct mutex s_lock; /* superblock lock */
>         struct mutex bitmap_lock; /* bitmap lock */
>         struct exfat_mount_options options;
> @@ -338,6 +343,11 @@ static inline struct exfat_inode_info *EXFAT_I(struct inode *inode)
>         return container_of(inode, struct exfat_inode_info, vfs_inode);
>  }
>
> +static inline int exfat_forced_shutdown(struct super_block *sb)
> +{
> +       return test_bit(EXFAT_FLAGS_SHUTDOWN, &EXFAT_SB(sb)->s_exfat_flags);
> +}
> +
>  /*
>   * If ->i_mode can't hold 0222 (i.e. ATTR_RO), we use ->i_attrs to
>   * save ATTR_RO instead of ->i_mode.
> diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c
> index dd894e558c91..b1b814183494 100644
> --- a/fs/exfat/inode.c
> +++ b/fs/exfat/inode.c
> @@ -452,6 +452,9 @@ static int exfat_write_begin(struct file *file, struct address_space *mapping,
>  {
>         int ret;
>
> +       if (unlikely(exfat_forced_shutdown(mapping->host->i_sb)))
> +               return -EIO;
We need to add this to other write operations(exfat_create, unlink,
mkdir, rmdir, rename, setattr, writepages)
as well as exfat_write_begin().
Thanks.