UBSAN: shift-out-of-bounds in exfat_fill_super

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 9791581c Merge tag 'for-5.11-rc4-tag' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10d94027500000
kernel config: https://syzkaller.appspot.com/x/.config?x=c5f3af3f625fb042
dashboard link: https://syzkaller.appspot.com/bug?extid=da4fe66aaadd3c2e2d1c
compiler: clang version 11.0.1
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11ad6f00d00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13ffb69f500000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+da4fe6...@syzkaller.appspotmail.com

loop0: detected capacity change from 16383 to 0
================================================================================
UBSAN: shift-out-of-bounds in fs/exfat/super.c:471:28
shift exponent 4294967294 is too large for 32-bit type 'int'
CPU: 0 PID: 8443 Comm: syz-executor876 Not tainted 5.11.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack+0x183/0x22e lib/dump_stack.c:120
ubsan_epilogue lib/ubsan.c:148 [inline]
__ubsan_handle_shift_out_of_bounds+0x432/0x4d0 lib/ubsan.c:395
exfat_read_boot_sector fs/exfat/super.c:471 [inline]
__exfat_fill_super fs/exfat/super.c:556 [inline]
exfat_fill_super+0x2acb/0x2d00 fs/exfat/super.c:624
get_tree_bdev+0x406/0x630 fs/super.c:1291
vfs_get_tree+0x86/0x270 fs/super.c:1496
do_new_mount fs/namespace.c:2881 [inline]
path_mount+0x1937/0x2c50 fs/namespace.c:3211
do_mount fs/namespace.c:3224 [inline]
__do_sys_mount fs/namespace.c:3432 [inline]
__se_sys_mount+0x2f9/0x3b0 fs/namespace.c:3409
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x446f8a
Code: b8 08 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 fd ad fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 0f 83 da ad fb ff c3 66 0f 1f 84 00 00 00 00 00
RSP: 002b:00007fff97c1b978 EFLAGS: 00000297 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007fff97c1b9d0 RCX: 0000000000446f8a
RDX: 0000000020000000 RSI: 0000000020000340 RDI: 00007fff97c1b990
RBP: 00007fff97c1b990 R08: 00007fff97c1b9d0 R09: 00007fff00000015
R10: 0000000000000000 R11: 0000000000000297 R12: 000000000000000c
R13: 0000000000000004 R14: 0000000000000003 R15: 0000000000000003
================================================================================


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Matthew Wilcox]

On Mon, Jan 25, 2021 at 09:33:14AM -0800, syzbot wrote:
> UBSAN: shift-out-of-bounds in fs/exfat/super.c:471:28
> shift exponent 4294967294 is too large for 32-bit type 'int'

This is an integer underflow:

sbi->dentries_per_clu = 1 <<
(sbi->cluster_size_bits - DENTRY_SIZE_BITS);

I think the problem is that there is no validation of sect_per_clus_bits.
We should check it is at least DENTRY_SIZE_BITS and probably that it's
less than ... 16? 64? I don't know what legitimate values are in this
field, but I would imagine that 255 is completely unacceptable.

[Randy Dunlap]

Ack all of that. The syzbot boot_sector has sect_per_clus_bits == 3
and sect_size_bits == 0, so sbi->cluster_size_bits is 3, then
UBSAN goes bang on:

sbi->dentries_per_clu = 1 <<
(sbi->cluster_size_bits - DENTRY_SIZE_BITS); // 3 - 5


There is also an unprotected shift at line 480:

if (sbi->num_FAT_sectors << p_boot->sect_size_bits <
sbi->num_clusters * 4) {

that should be protected IMO.


--
~Randy

[Namjae Jeon]

exfat specification describe sect_per_clus_bits field of boot sector could be at most 32 and
at least 0. And sect_size_bits can also affect this calculation, It also needs validation.
I will fix it.
Thanks!

[Namjae Jeon]

> exfat specification describe sect_per_clus_bits field of boot sector could be at most 32 and at least

typo ^^16

[Namjae Jeon]

Right. I will also add validation for fat_length as well as sect_size_bits before this.

Thanks!
>
>
> --
> ~Randy