lkml.org 
[lkml]   [2019]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] time64: Avoid undefined behaviour in timespec64_add()
From
Date


On 2/25/2019 12:53 PM, Deepa Dinamani wrote:
> 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.

Thanks for your reply.

> Are you running some kind of a fuzzer that would cause a overflow?

Yes, I am running syzkaller testsuite.

> 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.

Thanks for this suggestion. Looks like that is a better way.
I will try it.

Thanks,
HongBo



> -Deepa
>
> .
>

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