[PATCH] Tried making changes

[PATCH] Tried making changes

[attreyee-muk]

Respected Maintainers, 

I have tried to solve the bug - UBSAN: shift-out-of-bounds in exfat_fill_super, reported by Syzbot [link - https://syzkaller.appspot.com/bug?extid=d33808a177641a02213e]

Since it didn't have a reproducer, I wasn't able to test the patch
before sending to the maintainers.

The issue is in line 503 of fs/exfat/super.c - by analyzing the code, I
understood that the it is checking if the calculated size of the exFAT
File Allocation Table is very small as compared to the expected
size,based on the number of clusters. If the condition is met, then an
error will be logged. But here inside the if statement, I believe that
the value of number of bits in sbi->num_FAT_sectors ,at some point is
coming more than the value of p_boot->sect_size_bits. As a result, a
shift-out-of-bounds error is being generated. 

I tried using the hweight_long() to find the number of bits in
sbi->num_FAT_sectors in advance and then perform the left shift
operation only if it's total number of bits is greater than or equal to
the value of p_boot->sect_size_bits. 

I think that a new else statement should also be included with that and
I will do that once I get some help from the maintainers and get to know if I
am thinking in the right direction. 
Requesting the maintainers to go through the code once and kindly help me in understanding whether I am 
I am doing it wrong or if I need to add some more things. 

Thank you
Attreyee Mukherjee

Signed-off-by: Attreyee Mukherjee <tintinm2017@gmail.com>
---
 fs/exfat/super.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index d9d4fa91010b..0d526d9f3e5e 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -18,6 +18,7 @@
 #include <linux/nls.h>
 #include <linux/buffer_head.h>
 #include <linux/magic.h>
+#include <linux/bitops.h>
 
 #include "exfat_raw.h"
 #include "exfat_fs.h"
@@ -500,11 +501,18 @@ static int exfat_read_boot_sector(struct super_block *sb)
 	sbi->used_clusters = EXFAT_CLUSTERS_UNTRACKED;
 
 	/* check consistencies */
-	if ((u64)sbi->num_FAT_sectors << p_boot->sect_size_bits <
+	u64 num_fat_sectors_u64 = (u64)sbi->num_FAT_sectors;
+	unsigned long num_bits = hweight_long(num_fat_sectors_u64);
+
+	if(num_bits>=p_boot->sect_size_bits){
+
+		if ((u64)sbi->num_FAT_sectors << p_boot->sect_size_bits <
 	    (u64)sbi->num_clusters * 4) {
 		exfat_err(sb, "bogus fat length");
 		return -EINVAL;
+		}
 	}
+	
 
 	if (sbi->data_start_sector <
 	    (u64)sbi->FAT1_start_sector +
-- 
2.34.1

Re: [PATCH] Tried making changes

[Matthew Wilcox]

On Sat, Dec 02, 2023 at 10:09:00AM +0530, attreyee-muk wrote:
> Respected Maintainers, 
> 
> I have tried to solve the bug - UBSAN: shift-out-of-bounds in exfat_fill_super, reported by Syzbot [link - https://syzkaller.appspot.com/bug?extid=d33808a177641a02213e]

Hi Attreyee,

Working on syzbot reports is quite an art.  The important thing to know
for solving this one is that syzbot will fuzz filesystems.  That is, it
will start with a valid filesystem and change bits on disk, then see if
that creates any issues.

> The issue is in line 503 of fs/exfat/super.c - by analyzing the code, I
> understood that the it is checking if the calculated size of the exFAT
> File Allocation Table is very small as compared to the expected
> size,based on the number of clusters. If the condition is met, then an
> error will be logged. But here inside the if statement, I believe that
> the value of number of bits in sbi->num_FAT_sectors ,at some point is
> coming more than the value of p_boot->sect_size_bits. As a result, a
> shift-out-of-bounds error is being generated. 

No, that's not what's happening in this report.  p_boot->sect_size_bits
somehow has value 97.  And it's Undefined Behaviour in C to shift by more
than the number of bits in the type.  But I don't see how that happens:

fs/exfat/exfat_raw.h:#define EXFAT_MAX_SECT_SIZE_BITS           12

        if (p_boot->sect_size_bits < EXFAT_MIN_SECT_SIZE_BITS ||
            p_boot->sect_size_bits > EXFAT_MAX_SECT_SIZE_BITS) {

so something weird has happened; probably there's some other bug
somewhere else that has caused p_boot to be corrupted.  Whatever it is,
it's unlikely that you'll be able to find it.  Probably this is why
there's no reproducer.