On Wed, Jun 18, 2025 at 8:10 PM Yuezhang.Mo@xxxxxxxx
<Yuezhang.Mo@xxxxxxxx> wrote:
>
> > On Tue, Jun 17, 2025 at 2:38 PM Yuezhang.Mo@xxxxxxxx
> > <Yuezhang.Mo@xxxxxxxx> wrote:
> > >
> > > > 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().
> > Isn't the infinite loop issue in this patch improved by checking
> > clu_count and ->used_clusters?
> > If it reaches an unused entry, how about returning right away like before?
> >
>
> It reaches an unused directory entry and exits the loop, but it does not
> find that the cluster chain includes a loop. After using up these unused
> directory entries, an infinite loop will occur in exfat_check_dir_empty()
> and exfat_find_dir_entry(). To avoid this, it continues traversing to ensure
> that the cluster chain does not include a loop.
>
> For an un-corrupted file system, unused directory entries will only in the
> last cluster, so this only makes FAT is read one more time. What concerns
> do you have about this?
You described there is a potential infinite loop when there is no unused entry
in the patch description. There is an unused type check to break a loop in
exfat_check_dir_empty and exfat_find_dir_entry. I can not understand how
there can be an infinite loop if there is an unused entry. So there is no need
to strictly check the self-linked chain in exfat_count_dir_entries when there
is an unused entry. It can be treated as an error even though it can be
handled normally.
