On Tue, Aug 27, 2019 at 05:22:37PM +0200, Jan Kara wrote: > On Fri 12-07-19 10:28:37, SunKe wrote: > > 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@xxxxxxxxxx> I don't have the original mail in my mailbox to reply, let me qote the code here again: @@ -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; Can this be replaced by check_add_overflow? This can handle signed/unsigned types while the opencoding obscures the meaning. And a shameless plug, I sent a fix for another UB report, in remap_verify_area https://lore.kernel.org/lkml/20190808123942.19592-1-dsterba@xxxxxxxx/ that I'd like to get merged.
