Messages in this thread | | | From | Sasha Levin <> | Date | Wed, 14 Mar 2012 23:19:51 +0200 | Subject | Re: [PATCH] ntp: Fix integer overflow when setting time |
| |
On Wed, Mar 14, 2012 at 11:16 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Wed, 14 Mar 2012, Sasha Levin wrote: > >> On 64bit machines, the difference between 'time_reftime' and current time >> is 8 bits, this variable is used as the divisor in div_s64() to calculate >> the frequency, but div_s64() assumes the divisor is 32bits. > > Why is that delta 8 bits???? > >> This means that we are able to trigger a division by zero if the difference >> has 0's in it's lower bytes. This way it would skip the sanity checks before >> the div_64s() but when it gets to that div it would get truncated and >> become 0. > > This does not make sense at all. > > So what I assume you are talking about is, that the divisor (i.e. the > delta) is greater than (1 << 32) - 1 and all 32 lower bits are 0, right ?
Yup, exactly.
>> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c >> index 17fb1b9..efe894a 100644 >> --- a/kernel/time/ntp.c >> +++ b/kernel/time/ntp.c >> @@ -289,7 +289,7 @@ static inline s64 ntp_update_offset_fll(s64 offset64, long secs) >> >> time_status |= STA_MODE; >> >> - return div_s64(offset64 << (NTP_SCALE_SHIFT - SHIFT_FLL), secs); >> + return div64_s64(offset64 << (NTP_SCALE_SHIFT - SHIFT_FLL), secs); > > This needs a comment and it would be nice to avoid the 64 division on > 32bit machines. Something like: > > #if BITS_PER_LONG == 64 > # define div64_long(x,y) div64_s64(x,y) > #else > # define div64_long(x,y) div_s64(x,y) > #endif
I'll change that.
> Nice catch, though it took me a while to grok the changelog, as the 8 > bit case is catched by the MINSEC check. Please be more careful about > that.
Sorry about that, I suck at writing changelogs :(
> Thanks, > > tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |