Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1461364 > unrolled thread
| Started by | Vegard Nossum <vegard.nossum@oracle.com> |
|---|---|
| First post | 2016-08-12 20:20 +0200 |
| Last post | 2016-08-17 22:20 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] time: avoid undefined behaviour in timespec64_add_safe() Vegard Nossum <vegard.nossum@oracle.com> - 2016-08-12 20:20 +0200
Re: [PATCH] time: avoid undefined behaviour in timespec64_add_safe() John Stultz <john.stultz@linaro.org> - 2016-08-17 22:20 +0200
| From | Vegard Nossum <vegard.nossum@oracle.com> |
|---|---|
| Date | 2016-08-12 20:20 +0200 |
| Subject | [PATCH] time: avoid undefined behaviour in timespec64_add_safe() |
| Message-ID | <s5pdL-5cq-15@gated-at.bofh.it> |
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:
[<ffffffff82344f50>] dump_stack+0xac/0xfc
[<ffffffff82344ea4>] ? _atomic_dec_and_lock+0xc4/0xc4
[<ffffffff8242f4c8>] ubsan_epilogue+0xd/0x8a
[<ffffffff8242fc04>] handle_overflow+0x202/0x23d
[<ffffffff8242fa02>] ? val_to_string.constprop.6+0x11e/0x11e
[<ffffffff823c7837>] ? debug_smp_processor_id+0x17/0x20
[<ffffffff8131b581>] ? __sigqueue_free.part.13+0x51/0x70
[<ffffffff8146d4e0>] ? rcu_is_watching+0x110/0x110
[<ffffffff8242fc4d>] __ubsan_handle_add_overflow+0xe/0x10
[<ffffffff81476ef8>] timespec64_add_safe+0x298/0x340
[<ffffffff81476c60>] ? timespec_add_safe+0x330/0x330
[<ffffffff812f7990>] ? wait_noreap_copyout+0x1d0/0x1d0
[<ffffffff8184bf18>] poll_select_set_timeout+0xf8/0x170
[<ffffffff8184be20>] ? poll_schedule_timeout+0x2b0/0x2b0
[<ffffffff813aa9bb>] ? __might_sleep+0x5b/0x260
[<ffffffff833c8a87>] __sys_recvmmsg+0x107/0x790
[<ffffffff833c8980>] ? SyS_recvmsg+0x20/0x20
[<ffffffff81486378>] ? hrtimer_start_range_ns+0x3b8/0x1380
[<ffffffff845f8bfb>] ? _raw_spin_unlock_irqrestore+0x3b/0x60
[<ffffffff8148bcea>] ? do_setitimer+0x39a/0x8e0
[<ffffffff813aa9bb>] ? __might_sleep+0x5b/0x260
[<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790
[<ffffffff833c91e9>] SyS_recvmmsg+0xd9/0x160
[<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790
[<ffffffff823c7853>] ? __this_cpu_preempt_check+0x13/0x20
[<ffffffff8162f680>] ? __context_tracking_exit.part.3+0x30/0x1b0
[<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790
[<ffffffff81007bd3>] do_syscall_64+0x1b3/0x4b0
[<ffffffff845f936a>] 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).
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
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 7e5d2fa..980c71b 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -5,6 +5,7 @@
#include <linux/math64.h>
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 667b933..bd62fb8 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)) {
--
1.9.1
[toc] | [next] | [standalone]
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-08-17 22:20 +0200 |
| Message-ID | <s7ftE-5TG-9@gated-at.bofh.it> |
| In reply to | #1461364 |
On Fri, Aug 12, 2016 at 11:14 AM, Vegard Nossum <vegard.nossum@oracle.com> wrote: > 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: > [<ffffffff82344f50>] dump_stack+0xac/0xfc > [<ffffffff82344ea4>] ? _atomic_dec_and_lock+0xc4/0xc4 > [<ffffffff8242f4c8>] ubsan_epilogue+0xd/0x8a > [<ffffffff8242fc04>] handle_overflow+0x202/0x23d > [<ffffffff8242fa02>] ? val_to_string.constprop.6+0x11e/0x11e > [<ffffffff823c7837>] ? debug_smp_processor_id+0x17/0x20 > [<ffffffff8131b581>] ? __sigqueue_free.part.13+0x51/0x70 > [<ffffffff8146d4e0>] ? rcu_is_watching+0x110/0x110 > [<ffffffff8242fc4d>] __ubsan_handle_add_overflow+0xe/0x10 > [<ffffffff81476ef8>] timespec64_add_safe+0x298/0x340 > [<ffffffff81476c60>] ? timespec_add_safe+0x330/0x330 > [<ffffffff812f7990>] ? wait_noreap_copyout+0x1d0/0x1d0 > [<ffffffff8184bf18>] poll_select_set_timeout+0xf8/0x170 > [<ffffffff8184be20>] ? poll_schedule_timeout+0x2b0/0x2b0 > [<ffffffff813aa9bb>] ? __might_sleep+0x5b/0x260 > [<ffffffff833c8a87>] __sys_recvmmsg+0x107/0x790 > [<ffffffff833c8980>] ? SyS_recvmsg+0x20/0x20 > [<ffffffff81486378>] ? hrtimer_start_range_ns+0x3b8/0x1380 > [<ffffffff845f8bfb>] ? _raw_spin_unlock_irqrestore+0x3b/0x60 > [<ffffffff8148bcea>] ? do_setitimer+0x39a/0x8e0 > [<ffffffff813aa9bb>] ? __might_sleep+0x5b/0x260 > [<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790 > [<ffffffff833c91e9>] SyS_recvmmsg+0xd9/0x160 > [<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790 > [<ffffffff823c7853>] ? __this_cpu_preempt_check+0x13/0x20 > [<ffffffff8162f680>] ? __context_tracking_exit.part.3+0x30/0x1b0 > [<ffffffff833c9110>] ? __sys_recvmmsg+0x790/0x790 > [<ffffffff81007bd3>] do_syscall_64+0x1b3/0x4b0 > [<ffffffff845f936a>] 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). > > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> Queued for testing, targeting 4.9. thanks -john
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web