Re: [PATCH] timekeeping: Change type of nsec variable to unsigned in its calculation.

[Thomas Gleixner]

On Mon, 26 Sep 2016, Liav Rehana wrote:
> During the calculation of the nsec variable in the inline function
> timekeeping_delta_to_ns, it may undergo a sign extension if its msb
> is set just before the shift. The sign extension may, in some cases,
> gain it a value near the maximum value of the 64-bit range. This is
> bad when it is later used in a division function, such as
> __iter_div_u64_rem, where the amount of loops it will go through to
> calculate the division will be too large.
> The following commit fixes that chance of sign extension,

Again. This does not fix anything, it papers over the underlying problem
that the calling code hands in a delta which is big enough to overflow the
multiplication into the negative space. You just extend the range of deltas
which are handled gracefully, but that does not fix the underlying problem
as we still can run into the multiplication overflow. It won't cause the
result to be negative, but it will be crap nevertheless.

> while maintaining the type of the nsec variable as signed for other
> functions that use this variable, for possible legit negative time
> intervals.

What is this maintaining? The type of this variable changes to u64 and
other functions cannot use this variable at all because it's local to that
function. This sentence makes no sense at all.

Thanks,

	tglx