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) {
> + 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?
> + 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?
Thanks.
> + exfat_fs_error(sb, "The cluster chain has a loop");
> + return -EIO;
> + }
> +
> return 0;
> }
