Re: Bug: soft lockup in exfat_clear_bitmap

From: Namjae Jeon
Date: Wed Jan 22 2025 - 18:49:28 EST

On Wed, Jan 15, 2025 at 10:16 PM Kun Hu <huk23@xxxxxxxxxxxxxx> wrote:
>
>
> > This is an already known issue and the relevant patch has been applied.
> > Please make sure that the following patch is applied to the kernel you tested.
> >
> > a5324b3a488d exfat: fix the infinite loop in __exfat_free_cluster()
> >
> > or try to reproduce it with linux-6.13-rc7.
>
> Hi Namjae,
Hi Kun,
>
> We still successfully reproduced it on the v6.13-rc7. Firstly, I apologize for taking up your time, I’m not sure if this is a significant issue since from the reproducer it kind of looks like it’s caused via fault injection.
>
>
> The syz_mount_image in the syscall reproducer mounts a randomly generated image and also has the potential to trigger an abnormal path to the file system. Specifically, the . /file0 file is crafted to contain invalid FAT table or bitmap information, it is possible to cause abnormal cyclic behavior in __exfat_free_cluster.
>
> Because p_chain->size is artificially constructed, if it has a large value, then exfat_clear_bitmap will be called frequently. As the call stack shows, the program eventually deadlocks in the loop in __exfat_free_cluster.
>
> This link is a link to our crash log in the rc7 kernel tree:
>
> Link: https://github.com/pghk13/Kernel-Bug/blob/main/0103_6.13rc5_%E6%9C%AA%E6%8A%A5%E5%91%8A/%E6%9C%89%E7%9B%B8%E4%BC%BC%E6%A3%80%E7%B4%A2%E8%AE%B0%E5%BD%95/39-BUG_%20soft%20lockup%20in%20sys_unlink/crashlog0115_rc7.txt
>
> As I said earlier, I'm still consistently reporting the crash I found to you guys now because I'm not sure if this issue is useful to you. If it is not useful, please ignore it. I hope it doesn't take up too much of your time.
Can you check an attached patch ?

Thanks.
>
> ———
> Kun Hu
>
>
From 0bb26ac6aa65c9d9d41f19af305fb72c480fd1d6 Mon Sep 17 00:00:00 2001
From: Namjae Jeon <linkinjeon@kernel.org>
Date: Wed, 22 Jan 2025 00:24:31 +0900
Subject: [PATCH] exfat: fix infinite loop

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
fs/exfat/balloc.c | 10 ++++++++--
fs/exfat/exfat_fs.h | 2 +-
fs/exfat/fatent.c | 8 +++++---
3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/fs/exfat/balloc.c b/fs/exfat/balloc.c
index ce9be95c9172..9ff825f1502d 100644
--- a/fs/exfat/balloc.c
+++ b/fs/exfat/balloc.c
@@ -141,7 +141,7 @@ int exfat_set_bitmap(struct inode *inode, unsigned int clu, bool sync)
return 0;
}

-void exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync)
+int exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync)
{
int i, b;
unsigned int ent_idx;
@@ -150,13 +150,17 @@ void exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync)
struct exfat_mount_options *opts = &sbi->options;

if (!is_valid_cluster(sbi, clu))
- return;
+ return -EIO;

ent_idx = CLUSTER_TO_BITMAP_ENT(clu);
i = BITMAP_OFFSET_SECTOR_INDEX(sb, ent_idx);
b = BITMAP_OFFSET_BIT_IN_SECTOR(sb, ent_idx);

+ if (!test_bit_le(b, sbi->vol_amap[i]->b_data))
+ return -EIO;
+
clear_bit_le(b, sbi->vol_amap[i]->b_data);
+
exfat_update_bh(sbi->vol_amap[i], sync);

if (opts->discard) {
@@ -171,6 +175,8 @@ void exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync)
opts->discard = 0;
}
}
+
+ return 0;
}

/*
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 78be6964a8a0..d30ce18a88b7 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -456,7 +456,7 @@ int exfat_count_num_clusters(struct super_block *sb,
int exfat_load_bitmap(struct super_block *sb);
void exfat_free_bitmap(struct exfat_sb_info *sbi);
int exfat_set_bitmap(struct inode *inode, unsigned int clu, bool sync);
-void exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync);
+int exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync);
unsigned int exfat_find_free_bitmap(struct super_block *sb, unsigned int clu);
int exfat_count_used_clusters(struct super_block *sb, unsigned int *ret_count);
int exfat_trim_fs(struct inode *inode, struct fstrim_range *range);
diff --git a/fs/exfat/fatent.c b/fs/exfat/fatent.c
index 9e5492ac409b..8d78b3030575 100644
--- a/fs/exfat/fatent.c
+++ b/fs/exfat/fatent.c
@@ -175,6 +175,7 @@ static int __exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain
BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(clu));

if (p_chain->flags == ALLOC_NO_FAT_CHAIN) {
+ int err;
unsigned int last_cluster = p_chain->dir + p_chain->size - 1;
do {
bool sync = false;
@@ -189,7 +190,9 @@ static int __exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain
cur_cmap_i = next_cmap_i;
}

- exfat_clear_bitmap(inode, clu, (sync && IS_DIRSYNC(inode)));
+ err = exfat_clear_bitmap(inode, clu, (sync && IS_DIRSYNC(inode)));
+ if (err)
+ break;
clu++;
num_clusters++;
} while (num_clusters < p_chain->size);
@@ -215,7 +218,7 @@ static int __exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain
num_clusters++;

if (err)
- goto dec_used_clus;
+ break;

if (num_clusters >= sbi->num_clusters - EXFAT_FIRST_CLUSTER) {
/*
@@ -229,7 +232,6 @@ static int __exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain
} while (clu != EXFAT_EOF_CLUSTER);
}

-dec_used_clus:
sbi->used_clusters -= num_clusters;
return 0;
}
--
2.25.1