On Fri, 8 May 2015, John Stultz wrote: > It was noted that the 32bit implementation of ktime_divns() > was doing unsigned division and didn't properly handle > negative values. > > And when a ktime helper was changed to utilize > ktime_divns, it caused a regression on some IR blasters. > See the following bugzilla for details: > https://bugzilla.redhat.com/show_bug.cgi?id=1200353 > > This patch fixes the problem in ktime_divns by checking > and preserving the sign bit, and then reapplying it if > appropriate after the division, it also changes the return > type to a s64 to make it more obvious this is expected. > > Nicolas also pointed out that negative dividers would > cause infinite loops on 32bit systems, negative dividers > is unlikely for users of this function, but out of caution > this patch adds checks for negative dividers for both > 32-bit (BUG_ON) and 64-bit(WARN_ON) versions to make sure > no such use cases creep in. > > Cc: Nicolas Pitre <nicolas.pitre@xxxxxxxxxx> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Cc: Ingo Molnar <mingo@xxxxxxxxxx> > Cc: Josh Boyer <jwboyer@xxxxxxxxxx> > Cc: One Thousand Gnomes <gnomes@xxxxxxxxxxxxxxxxxxx> > Cc: Trevor Cordes <trevor@xxxxxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> # 3.17+ for regression > Tested-by: Trevor Cordes <trevor@xxxxxxxxxxxxx> > Reported-by: Trevor Cordes <trevor@xxxxxxxxxxxxx> > Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx> Acked-by: Nicolas Pitre <nico@xxxxxxxxxx> > --- > > New in v3: > * Fix casting issue Nicolas pointed out > * Use WARN_ON for 64bit case > > include/linux/ktime.h | 27 +++++++++++++++++++++++---- > kernel/time/hrtimer.c | 11 ++++++++--- > 2 files changed, 31 insertions(+), 7 deletions(-) > > diff --git a/include/linux/ktime.h b/include/linux/ktime.h > index 5fc3d10..ab2de1c7 100644 > --- a/include/linux/ktime.h > +++ b/include/linux/ktime.h > @@ -166,19 +166,38 @@ static inline bool ktime_before(const ktime_t cmp1, const ktime_t cmp2) > } > > #if BITS_PER_LONG < 64 > -extern u64 __ktime_divns(const ktime_t kt, s64 div); > -static inline u64 ktime_divns(const ktime_t kt, s64 div) > +extern s64 __ktime_divns(const ktime_t kt, s64 div); > +static inline s64 ktime_divns(const ktime_t kt, s64 div) > { > + /* > + * Negative divisors could cause an inf loop, > + * so bug out here. > + */ > + BUG_ON(div < 0); > if (__builtin_constant_p(div) && !(div >> 32)) { > - u64 ns = kt.tv64; > + s64 ns = kt.tv64; > + int neg = (ns < 0); > + > + if (neg) > + ns = -ns; > do_div(ns, div); > + if (neg) > + ns = -ns; > return ns; > } else { > return __ktime_divns(kt, div); > } > } > #else /* BITS_PER_LONG < 64 */ > -# define ktime_divns(kt, div) (u64)((kt).tv64 / (div)) > +static inline s64 ktime_divns(const ktime_t kt, s64 div) > +{ > + /* > + * 32-bit implementation cannot handle negative divisors, > + * so catch them on 64bit as well. > + */ > + WARN_ON(div < 0); > + return kt.tv64 / div; > +} > #endif > > static inline s64 ktime_to_us(const ktime_t kt) > diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c > index 76d4bd9..c98ce4d 100644 > --- a/kernel/time/hrtimer.c > +++ b/kernel/time/hrtimer.c > @@ -266,12 +266,15 @@ lock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags) > /* > * Divide a ktime value by a nanosecond value > */ > -u64 __ktime_divns(const ktime_t kt, s64 div) > +s64 __ktime_divns(const ktime_t kt, s64 div) > { > - u64 dclc; > - int sft = 0; > + s64 dclc; > + int neg, sft = 0; > > dclc = ktime_to_ns(kt); > + neg = (dclc < 0); > + if (neg) > + dclc = -dclc; > /* Make sure the divisor is less than 2^32: */ > while (div >> 32) { > sft++; > @@ -279,6 +282,8 @@ u64 __ktime_divns(const ktime_t kt, s64 div) > } > dclc >>= sft; > do_div(dclc, (unsigned long) div); > + if (neg) > + dclc = -dclc; > > return dclc; > } > -- > 1.9.1 > > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html