[PATCH] hugetlbfs: end hpage in hugetlbfs_fallocate overflow

[PATCH] hugetlbfs: end hpage in hugetlbfs_fallocate overflow

[luojiajun]

In hugetlbfs_fallocate, start is rounded down and end is rounded up.
But it is inappropriate to use loff_t rounding up end, it may cause
overflow.

UBSAN: Undefined behaviour in fs/hugetlbfs/inode.c:582:22
signed integer overflow:
2097152 + 9223372036854775805 cannot be represented in type 'long long int'
CPU: 0 PID: 2669 Comm: syz-executor662 Not tainted 4.19.30 #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
BIOS 1.10.2-1ubuntu1 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xca/0x13e lib/dump_stack.c:113
 ubsan_epilogue+0xe/0x81 lib/ubsan.c:159
 handle_overflow+0x193/0x1e2 lib/ubsan.c:190
 hugetlbfs_fallocate+0xe72/0x1140 fs/hugetlbfs/inode.c:582
 vfs_fallocate+0x346/0x7a0 fs/open.c:308
 ioctl_preallocate+0x15d/0x200 fs/ioctl.c:482
 file_ioctl fs/ioctl.c:498 [inline]
 do_vfs_ioctl+0xde3/0x10a0 fs/ioctl.c:688
 ksys_ioctl+0x89/0xa0 fs/ioctl.c:705
 __do_sys_ioctl fs/ioctl.c:712 [inline]
 __se_sys_ioctl fs/ioctl.c:710 [inline]
 __x64_sys_ioctl+0x74/0xb0 fs/ioctl.c:710
 do_syscall_64+0xc8/0x580 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x44a3e9

Fix this problem by casting loff_t to unsigned long long when end
is rounded up.

This problem can be reproduced by syzkaller

Signed-off-by: luojiajun <luojiajun3@huawei.com>
---
 fs/hugetlbfs/inode.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 7f33244..0fe07f2 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -578,8 +578,9 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
 	 * For this range, start is rounded down and end is rounded up
 	 * as well as being converted to page offsets.
 	 */
-	start = offset >> hpage_shift;
-	end = (offset + len + hpage_size - 1) >> hpage_shift;
+	start = (unsigned long long)offset >> hpage_shift;
+	end = ((unsigned long long)(offset + len + hpage_size) - 1)
+			>> hpage_shift;
 
 	inode_lock(inode);
 
-- 
2.7.4

Re: [PATCH] hugetlbfs: end hpage in hugetlbfs_fallocate overflow

[Matthew Wilcox]

On Tue, Apr 09, 2019 at 10:00:26AM +0800, luojiajun wrote:
> In hugetlbfs_fallocate, start is rounded down and end is rounded up.
> But it is inappropriate to use loff_t rounding up end, it may cause
> overflow.
> 
> UBSAN: Undefined behaviour in fs/hugetlbfs/inode.c:582:22
> signed integer overflow:
> 2097152 + 9223372036854775805 cannot be represented in type 'long long int'

This patch can't fix this bug.

> @@ -578,8 +578,9 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
>  	 * For this range, start is rounded down and end is rounded up
>  	 * as well as being converted to page offsets.
>  	 */
> -	start = offset >> hpage_shift;
> -	end = (offset + len + hpage_size - 1) >> hpage_shift;
> +	start = (unsigned long long)offset >> hpage_shift;
> +	end = ((unsigned long long)(offset + len + hpage_size) - 1)
> +			>> hpage_shift;

I suspect you mean:

	end = (((unsigned long long)offset + len + hpage_size) - 1) >>
			hpage_shift;

Otherwise, you're going to do the arithmetic in long long, then cast
to unsigned long long before the shift.

BTW, don't say "this can be reproduced using syzcaller".  This is an easy
case to extract a small reproducer from ... which would have helped you
notice that you haven't fixed the problem.