> On Fri, Jun 13, 2025 at 7:39 PM Yuezhang Mo <Yuezhang.Mo@xxxxxxxx> wrote:
> >
> > An infinite loop may occur if the following conditions occur due to
> > file system corruption.
> >
> > (1) Condition for exfat_count_dir_entries() to loop infinitely.
> > - The cluster chain includes a loop.
> > - There is no UNUSED entry in the cluster chain.
> >
> > (2) Condition for exfat_create_upcase_table() to loop infinitely.
> > - The cluster chain of the root directory includes a loop.
> > - There are no UNUSED entry and up-case table entry in the cluster
> > chain of the root directory.
> >
> > (3) Condition for exfat_load_bitmap() to loop infinitely.
> > - The cluster chain of the root directory includes a loop.
> > - There are no UNUSED entry and bitmap entry in the cluster chain
> > of the root directory.
> >
> > This commit adds checks in exfat_count_num_clusters() and
> > exfat_count_dir_entries() to see if the cluster chain includes a loop,
> > thus avoiding the above infinite loops.
> >
> > Signed-off-by: Yuezhang Mo <Yuezhang.Mo@xxxxxxxx>
> > ---
> > fs/exfat/dir.c | 33 +++++++++++++++++++++------------
> > fs/exfat/fatent.c | 10 ++++++++++
> > fs/exfat/super.c | 32 +++++++++++++++++++++-----------
> > 3 files changed, 52 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
> > index 3103b932b674..467271ad4d71 100644
> > --- a/fs/exfat/dir.c
> > +++ b/fs/exfat/dir.c
> > @@ -1194,7 +1194,8 @@ int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir)
> > {
> > int i, count = 0;
> > int dentries_per_clu;
> > - unsigned int entry_type;
> > + unsigned int entry_type = TYPE_FILE;
> > + unsigned int clu_count = 0;
> > struct exfat_chain clu;
> > struct exfat_dentry *ep;
> > struct exfat_sb_info *sbi = EXFAT_SB(sb);
> > @@ -1205,18 +1206,26 @@ int exfat_count_dir_entries(struct super_block *sb, struct exfat_chain *p_dir)
> > exfat_chain_dup(&clu, p_dir);
> >
> > while (clu.dir != EXFAT_EOF_CLUSTER) {
> > - for (i = 0; i < dentries_per_clu; i++) {
> > - ep = exfat_get_dentry(sb, &clu, i, &bh);
> > - if (!ep)
> > - return -EIO;
> > - entry_type = exfat_get_entry_type(ep);
> > - brelse(bh);
> > + clu_count++;
> > + if (clu_count > sbi->used_clusters) {
> if (++clu_count > sbi->used_clusters) {
Well, that's more concise.
> > + exfat_fs_error(sb, "dir size or FAT or bitmap is corrupted");
> > + return -EIO;
> > + }
> >
> > - if (entry_type == TYPE_UNUSED)
> > - return count;
> > - if (entry_type != TYPE_DIR)
> > - continue;
> > - count++;
> > + if (entry_type != TYPE_UNUSED) {
> > + for (i = 0; i < dentries_per_clu; i++) {
> > + ep = exfat_get_dentry(sb, &clu, i, &bh);
> > + if (!ep)
> > + return -EIO;
> > + entry_type = exfat_get_entry_type(ep);
> > + brelse(bh);
> > +
> > + if (entry_type == TYPE_UNUSED)
> > + break;
> Is there any reason why you keep doing loop even though you found an
> unused entry?
There are unused directory entries when calling this func, but there
may be none after files are created. That will cause a infinite loop in
exfat_check_dir_empty() and exfat_find_dir_entry().
>
> > + if (entry_type != TYPE_DIR)
> > + continue;
> > + count++;
> > + }
> > }
> >
> > if (clu.flags == ALLOC_NO_FAT_CHAIN) {
> > diff --git a/fs/exfat/fatent.c b/fs/exfat/fatent.c
> > index 23065f948ae7..2a2615ca320f 100644
> > --- a/fs/exfat/fatent.c
> > +++ b/fs/exfat/fatent.c
> > @@ -490,5 +490,15 @@ int exfat_count_num_clusters(struct super_block *sb,
> > }
> >
> > *ret_count = count;
> > +
> > + /*
> > + * since exfat_count_used_clusters() is not called, sbi->used_clusters
> > + * cannot be used here.
> > + */
> > + if (i == sbi->num_clusters) {
> This is also right, But to make it more clear, wouldn't it be better
> to do clu != EXFAT_EOF_CLUSTER?
OK, I will add this.
>
> Thanks.
> > + exfat_fs_error(sb, "The cluster chain has a loop");
> > + return -EIO;
> > + }
> > +
> > return 0;
> > }
