From 469e857f374640f6164913835ce30d0736b40a60 Mon Sep 17 00:00:00 2001 From: Vegard Nossum Date: Fri, 12 Aug 2016 20:14:09 +0200 Subject: [PATCH] time: Avoid undefined behaviour in timespec64_add_safe() I ran into this: ================================================================================ UBSAN: Undefined behaviour in kernel/time/time.c:783:2 signed integer overflow: 5273 + 9223372036854771711 cannot be represented in type 'long int' CPU: 0 PID: 17363 Comm: trinity-c0 Not tainted 4.8.0-rc1+ #88 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014 0000000000000000 ffff88011457f8f0 ffffffff82344f50 0000000041b58ab3 ffffffff84f98080 ffffffff82344ea4 ffff88011457f918 ffff88011457f8c8 ffff88011457f8e0 7fffffffffffefff ffff88011457f6d8 dffffc0000000000 Call Trace: [] dump_stack+0xac/0xfc [] ? _atomic_dec_and_lock+0xc4/0xc4 [] ubsan_epilogue+0xd/0x8a [] handle_overflow+0x202/0x23d [] ? val_to_string.constprop.6+0x11e/0x11e [] ? debug_smp_processor_id+0x17/0x20 [] ? __sigqueue_free.part.13+0x51/0x70 [] ? rcu_is_watching+0x110/0x110 [] __ubsan_handle_add_overflow+0xe/0x10 [] timespec64_add_safe+0x298/0x340 [] ? timespec_add_safe+0x330/0x330 [] ? wait_noreap_copyout+0x1d0/0x1d0 [] poll_select_set_timeout+0xf8/0x170 [] ? poll_schedule_timeout+0x2b0/0x2b0 [] ? __might_sleep+0x5b/0x260 [] __sys_recvmmsg+0x107/0x790 [] ? SyS_recvmsg+0x20/0x20 [] ? hrtimer_start_range_ns+0x3b8/0x1380 [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 [] ? do_setitimer+0x39a/0x8e0 [] ? __might_sleep+0x5b/0x260 [] ? __sys_recvmmsg+0x790/0x790 [] SyS_recvmmsg+0xd9/0x160 [] ? __sys_recvmmsg+0x790/0x790 [] ? __this_cpu_preempt_check+0x13/0x20 [] ? __context_tracking_exit.part.3+0x30/0x1b0 [] ? __sys_recvmmsg+0x790/0x790 [] do_syscall_64+0x1b3/0x4b0 [] entry_SYSCALL64_slow_path+0x25/0x25 ================================================================================ Line 783 is this: 783 set_normalized_timespec64(&res, lhs.tv_sec + rhs.tv_sec, 784 lhs.tv_nsec + rhs.tv_nsec); In other words, since lhs.tv_sec and rhs.tv_sec are both time64_t, this is a signed addition which will cause undefined behaviour on overflow. Note that this is not currently a huge concern since the kernel should be built with -fno-strict-overflow by default, but could be a problem in the future, a problem with older compilers, or other compilers than gcc. The easiest way to avoid the overflow is to cast one of the arguments to unsigned (so the addition will be done using unsigned arithmetic). Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Richard Cochran Cc: Prarit Bhargava Signed-off-by: Vegard Nossum Signed-off-by: John Stultz --- include/linux/time64.h | 1 + kernel/time/time.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/time64.h b/include/linux/time64.h index 7e5d2fa9ac46..980c71b3001a 100644 --- a/include/linux/time64.h +++ b/include/linux/time64.h @@ -5,6 +5,7 @@ #include typedef __s64 time64_t; +typedef __u64 timeu64_t; /* * This wants to go into uapi/linux/time.h once we agreed about the diff --git a/kernel/time/time.c b/kernel/time/time.c index 667b9335f5d6..bd62fb8e8e77 100644 --- a/kernel/time/time.c +++ b/kernel/time/time.c @@ -780,7 +780,7 @@ struct timespec64 timespec64_add_safe(const struct timespec64 lhs, { struct timespec64 res; - set_normalized_timespec64(&res, lhs.tv_sec + rhs.tv_sec, + set_normalized_timespec64(&res, (timeu64_t) lhs.tv_sec + rhs.tv_sec, lhs.tv_nsec + rhs.tv_nsec); if (unlikely(res.tv_sec < lhs.tv_sec || res.tv_sec < rhs.tv_sec)) { -- 2.39.5