> > > The compatibility issue between linux exfat and exfat of some camera
> > > company was reported from Florian. In their exfat, if the number of
> > > files exceeds any limit, the DataLength in stream entry of the
> > > directory is no longer updated. So some files created from camera
> > > does not show in linux exfat. because linux exfat doesn't allow that
> > > cpos becomes larger than DataLength
> > of stream entry. This patch check DataLength in stream entry only if
> > the type is ALLOC_NO_FAT_CHAIN and add the check ensure that dentry
> > offset does not exceed max dentries size(256 MB) to avoid the circular FAT chain issue.
> >
> > Instead of using fsd to handle this, shouldn't it be left to fsck?
> Yes, That's what I thought at first. And fsck.exfat in exfatprogs can detect it like this.
>
> $ fsck.exfat /dev/sdb1
> exfatprogs version : 1.1.1
> ERROR: /DCIM/344_FUJI: more clusters are allocated. truncate to 524288 bytes. Truncate (y/N)? n
>
> >
> > In the exfat specification says, the DataLength Field of the
> > directory-stream is the entire size of the associated allocation.
> > If the DataLength Field does not match the size in the FAT-chain, it means that it is corrupted.
> Yes. I have checked it.
> >
> > As you know, the FAT-chain structure is fragile.
> > At runtime, one way to detect a broken FAT-chain is to compare it with DataLength.
> > (Detailed verification is the role of fsck).
> > Ignoring DataLength during dir-scan is unsafe because we lose a way to detect a broken FAT-chain.
> >
> > I think fsd should check DataLength, and fsck should repair DataLength.
> But Windows fsck doesn’t detect it and it shows the all files normally without any missing ones.
> It means Windows exfat doesn't also check it in case type is ALLOC_FAT_CHAIN.
Okay, I get it.
Because if our driver aborts scanning, the files after that will not be visible.
If FAT-Chain is correct and DataLenth is incorrect, this patch works safely.(as the case of Fuji)
However, if DataLenth is correct and FAT-Chain is incorrect, it is unsafe.
There is a risk of destroying other files/dirs.
The current implementation sets SB_RDONLY when it detects a directories FAT-Chain count and DataLenth contradiction in exfat_find_last_cluster( ).
It only works only on exfat_add_entry()/exfat_rename_file()/exfat_move_file().
Why don't we do a similar check for exfat_readdir()/exfat_find_dir_entry()?
Just do exfat_fs_error() if the dentry position exceeds DataLenth in the scan loop.
This will reduces the risk of destroying other files/dirs and prompts the user for fsck.
> Fixes: ca06197382bd ("exfat: add directory operations")
> Cc: stable@xxxxxxxxxxxxxxx # v5.9
> Reported-by: Florian Cramer <flrncrmr@xxxxxxxxx>
> Reviewed-by: Sungjong Seo <sj1557.seo@xxxxxxxxxxx>
> Signed-off-by: Namjae Jeon <namjae.jeon@xxxxxxxxxxx>
> ---
> fs/exfat/dir.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index c4523648472a..f4e4d8d9894d 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -63,7 +63,7 @@ static void exfat_get_uniname_from_ext_entry(struct super_block *sb, static int
> exfat_readdir(struct inode *inode, loff_t *cpos, struct exfat_dir_entry *dir_entry) {
> int i, dentries_per_clu, dentries_per_clu_bits = 0, num_ext;
> - unsigned int type, clu_offset;
> + unsigned int type, clu_offset, max_dentries;
> sector_t sector;
> struct exfat_chain dir, clu;
> struct exfat_uni_name uni_name;
> @@ -86,6 +86,8 @@ static int exfat_readdir(struct inode *inode, loff_t *cpos, struct exfat_dir_ent
>
> dentries_per_clu = sbi->dentries_per_clu;
> dentries_per_clu_bits = ilog2(dentries_per_clu);
> + max_dentries = (unsigned int)min_t(u64, MAX_EXFAT_DENTRIES,
> + (u64)sbi->num_clusters << dentries_per_clu_bits);
>
> clu_offset = dentry >> dentries_per_clu_bits;
> exfat_chain_dup(&clu, &dir);
> @@ -109,7 +111,7 @@ static int exfat_readdir(struct inode *inode, loff_t *cpos, struct exfat_dir_ent
> }
> }
>
> - while (clu.dir != EXFAT_EOF_CLUSTER) {
> + while (clu.dir != EXFAT_EOF_CLUSTER && dentry < max_dentries) {
> i = dentry & (dentries_per_clu - 1);
>
> for ( ; i < dentries_per_clu; i++, dentry++) { @@ -245,7 +247,7 @@ static int exfat_iterate(struct file *filp,
> struct dir_context *ctx)
> if (err)
> goto unlock;
> get_new:
> - if (cpos >= i_size_read(inode))
> + if (ei->flags == ALLOC_NO_FAT_CHAIN && cpos >= i_size_read(inode))
> goto end_of_dir;
This check is no longer necessary.
In the case of ALLOC_NO_FAT_CHAIN, there is a check for i_size in exfat_readdir(), which is a double check.
In the past, it was possible to skip chain tracking in the case of ALLOC_FAT_CHAIN, but that effect is also gone.
>
> err = exfat_readdir(inode, &cpos, &de);
BR
Kohada Tetsuhiro <Kohada.Tetsuhiro@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
