lkml.org 
[lkml]   [2019]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] time64: Avoid undefined behaviour in timespec64_add()
On Sun, Feb 24, 2019 at 7:13 PM Hongbo Yao <yaohongbo@huawei.com> wrote:
>
> I ran into this:
> =========================================================================
> UBSAN: Undefined behaviour in ./include/linux/time64.h:70:2
> signed integer overflow:
> 1551059291 + 9223372036854775807 cannot be represented in type 'long
> long int'
> CPU: 5 PID: 20064 Comm: syz-executor.2 Not tainted 4.19.24 #4
> 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
> timespec64_add include/linux/time64.h:70 [inline]
> timekeeping_inject_offset+0x3ed/0x4e0 kernel/time/timekeeping.c:1301
> do_adjtimex+0x1e5/0x6c0 kernel/time/timekeeping.c:2360
> __do_sys_clock_adjtime+0x122/0x200 kernel/time/posix-timers.c:1086
> do_syscall_64+0xc8/0x580 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x462eb9
> Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89
> f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01
> f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007f888aa2dc58 EFLAGS: 00000246 ORIG_RAX: 0000000000000131
> RAX: ffffffffffffffda RBX: 000000000073bf00 RCX: 0000000000462eb9
> RDX: 0000000000000000 RSI: 00000000200003c0 RDI: 0000000000000000
> RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007f888aa2e6bc
> R13: 00000000004bcae8 R14: 00000000006f6868 R15: 00000000ffffffff
> ==========================================================================
>
> Since lhs.tv_sec and rhs.tv_sec are both time64_t, this is a signed
> addition which will cause undefined behaviour on overflow.
>
> 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).
> This patch doesn't change generated code.
>
> Signed-off-by: Hongbo Yao <yaohongbo@huawei.com>
> ---
> include/linux/time64.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/time64.h b/include/linux/time64.h
> index 05634afba0db..5926bdd4167f 100644
> --- a/include/linux/time64.h
> +++ b/include/linux/time64.h
> @@ -67,7 +67,7 @@ static inline struct timespec64 timespec64_add(struct timespec64 lhs,
> struct timespec64 rhs)
> {
> struct timespec64 ts_delta;
> - set_normalized_timespec64(&ts_delta, lhs.tv_sec + rhs.tv_sec,
> + set_normalized_timespec64(&ts_delta, (timeu64_t)lhs.tv_sec + rhs.tv_sec,
> lhs.tv_nsec + rhs.tv_nsec);
> return ts_delta;
> }

There is already a timespec64_add_safe() to account for such
overflows. That assumes both the timespec64 values are positive.
But, timekeeping_inject_offset() cannot use that as one of the values
can be negative.

Are you running some kind of a fuzzer that would cause a overflow?
You seem to be adding INT64_MAX here. Maybe the right thing to do is
to add a check at the syscall interface rather than here.

-Deepa

\
 
 \ /
  Last update: 2019-02-25 05:55    [W:0.074 / U:6.304 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site