Re: [PATCH] Fix UBSAN warning for subtracting ktime_t

[Weiß, Simone]

On Fri, 2024-01-12 at 17:14 +0100, Thomas Gleixner wrote:
> CAUTION: This email originated from outside of the Elektrobit organization. Do
> not click links or open attachments unless you recognize the sender and know
> the content is safe.
> 
> 
> On Tue, Dec 19 2023 at 13:44, Simone Weiss wrote:
> > UBSAN: Undefined behaviour in kernel/time/hrtimer.c:612:10
> > signed integer overflow:
> > 9223372036854775807 - -51224496 cannot be represented in type
> > 'long long int'
> 
> Some explanation about the context and why the offset is < 0 would be
> helpful.
> 
> > +/*
> > + * Sub two ktime values and do a safety check for overflow:
> > + */
> > +ktime_t ktime_sub_safe(const ktime_t lhs, const ktime_t rhs)
> > +{
> > +     ktime_t res = ktime_sub_unsafe(lhs, rhs);
> > +
> > +     if (lhs > 0 && rhs < 0 && res < 0)
> > +             res = ktime_set(KTIME_SEC_MAX, 0);
> > +     else if (lhs < 0 && rhs > 0 && res > 0)
> > +             res = ktime_set(-KTIME_SEC_MAX, 0);
> 
> Looking at the use cases, the lhs < 0 case would be a bug because the
> expiry values are strictly >= 0.
> 
> > +
> > +     return res;
> > +}
> > +EXPORT_SYMBOL_GPL(ktime_sub_safe);
> 
> That export is needed because? The only usage is in this file so this
> can be static, no?
> 
> Thanks,
> 
>         tglx

Hi,

after more investigation it seems to me that this issue is only present in older
kernels like 4.14.y or 4.19.y. A stack trace I obtained from fuzzing 4.14 is:

Hardware name: linux,dummy-virt (DT)
Call trace:
 dump_backtrace+0x0/0x330
 show_stack+0x20/0x30 arch/arm64/kernel/traps.c:156
 __dump_stack lib/dump_stack.c:17 [inline]
 dump_stack+0x12c/0x180 lib/dump_stack.c:58
 ubsan_epilogue+0x18/0x50 lib/ubsan.c:166
 handle_overflow+0xf4/0x118 lib/ubsan.c:197
 __ubsan_handle_sub_overflow+0x34/0x48 lib/ubsan.c:211
 hrtimer_reprogram kernel/time/hrtimer.c:612 [inline]
 hrtimer_start_range_ns+0x768/0x858 kernel/time/hrtimer.c:993
 hrtimer_start include/linux/hrtimer.h:377 [inline]
 timerfd_setup fs/timerfd.c:205 [inline]
 do_timerfd_settime fs/timerfd.c:496 [inline]
 SYSC_timerfd_settime fs/timerfd.c:544 [inline]
 SyS_timerfd_settime+0x4c0/0x7e0 fs/timerfd.c:535
 el0_svc_naked+0x34/0x38
My present assumption is, that it is already fixed in newer kernel versions
via this commit:
6cd889d43c40b ("timerfd: Make timerfd_settime() time namespace aware").

I will check further.

Best,
Simone