tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Portability fix in kern_ntptime.c

On 05.06.2019 23:42, Robert Elz wrote:
>     Date:        Wed, 5 Jun 2019 22:25:39 +0200
>     From:        Kamil Rytarowski <>
>     Message-ID:  <>
>   | Does this patch look good?
> Personally, I would hesitate to change any of the NTP related
> code without an extremely good reason, and without a lot of
> testing first (in all kinds of conditions).
> Further, I'd never do it without a thorough review of the code,
> if you looked, you'd also see
>                 freq = (ntv->freq * 1000LL) >> 16;
> and
>         ntv->ppsfreq = L_GINT((pps_freq / 1000LL) << 16);
> (and perhaps more) - if one of those is a shift of a negative number,
> the others potentially are as well (not that shifts of negative numbers
> bother me at all if the code author understood what is happening, which
> I suspect that they did here.)
> kre

This code looks suspicious as kernl_ntptime.c in general looks like
trying to prevent UB in shift operations... but not fully.

The L_GINT() macro handles negative values... but someone (?) overlooked
that the passed argument is shifted for negative values. In my patch,
I'm assuming that this can happen legitimately (and it does happen) and
I'm replacing it with a portable form. There is an option to keep the
shift there but use casts:

ntv->freq = L_GINT((int64_t)((uint64_t)(time_freq / 1000LL) << 16));

I can switch all the other places of this code in one go.

Attachment: signature.asc
Description: OpenPGP digital signature

Home | Main Index | Thread Index | Old Index