linux-kernel - [PATCH] fs/sync.c: Fix UBSAN Undefined behaviour in sync_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 12 Jul 2019 10:28:37 +0800
From:   SunKe <sunke32@...wei.com>
To:     <viro@...iv.linux.org.uk>, <linux-fsdevel@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <sunke32@...wei.com>
Subject: [PATCH] fs/sync.c: Fix UBSAN Undefined behaviour in sync_file_range

There is a UBSAN report:
UBSAN: Undefined behaviour in ../fs/sync.c:298:10
signed integer overflow:
-8 + -9223372036854775807 cannot be represented in type 'long long int'
CPU: 0 PID: 15876 Comm: syz-executor.3 Not tainted
Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
Call trace:
[<ffffff90080ac450>] dump_backtrace+0x0/0x698 arch/arm64/kernel/traps.c:96
[<ffffff90080acb20>] show_stack+0x38/0x60 arch/arm64/kernel/traps.c:234
[<ffffff9008ca4500>] __dump_stack lib/dump_stack.c:15 [inline]
[<ffffff9008ca4500>] dump_stack+0x1a8/0x230 lib/dump_stack.c:51
[<ffffff9008d7e078>] ubsan_epilogue+0x34/0x9c lib/ubsan.c:164
[<ffffff9008d7ebb4>] handle_overflow+0x228/0x280 lib/ubsan.c:195
[<ffffff9008d7ed28>] __ubsan_handle_add_overflow+0x4c/0x68 lib/ubsan.c:203
[<ffffff900874c2b8>] SYSC_sync_file_range fs/sync.c:298 [inline]
[<ffffff900874c2b8>] SyS_sync_file_range+0x350/0x3e8 fs/sync.c:285
[<ffffff9008094480>] el0_svc_naked+0x30/0x34

When calculate the endbyte, there maybe an overflow, even if no effect
the kernel, but I also want to avoid overflowing and avoid UBSAN reporting.
The original compare is to ensure the offset >= 0 && nbytes >= 0 && no
overflow happened.

I do the calculate after compare. ensure the offset >= 0 && nbytes >= 0 &&
no overflow may happen first.

Signed-off-by: SunKe <sunke32@...wei.com>
---
 fs/sync.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/sync.c b/fs/sync.c
index 4d1ff01..5827471 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -246,15 +246,15 @@ int sync_file_range(struct file *file, loff_t offset, loff_t nbytes,
 	if (flags & ~VALID_FLAGS)
 		goto out;
 
-	endbyte = offset + nbytes;
-
 	if ((s64)offset < 0)
 		goto out;
-	if ((s64)endbyte < 0)
+	if ((s64)nbytes < 0)
 		goto out;
-	if (endbyte < offset)
+	if (S64_MAX - offset < nbytes)
 		goto out;
 
+	endbyte = offset + nbytes;
+
 	if (sizeof(pgoff_t) == 4) {
 		if (offset >= (0x100000000ULL << PAGE_SHIFT)) {
 			/*
-- 
2.7.4