[Nagiosplug-devel] Flight 1.4.8, ready for boarding

[Thomas Guyot-Sionnest]

On 31/03/07 02:53 PM, Holger Weiss wrote:
 > Your patch should be committed in any case.

I'll do.

> What is the exact message you get?  The message "warning: unable to read
> server jitter response" indicates that the server simply doesn't support
> jitter control packets.  However, with all servers I tried, the plugin
> returns a WARNING state _without_ explaining the problem:
> 
> | $ ./check_ntp -H time.fu-berlin.de -j 2 -k 3
> | NTP WARNING: Offset 0.05885159969 secs|offset=0.05885159969 jitter=0.001171
> 
> This WARNING is issued in jitter_request():
> 
> | if(! syncsource_found) *status = STATE_WARNING;
> 
> I'm not familiar with NTP and haven't tracked down whether the plugin
> does something wrong while checking for the synchronization source; and
> if not, whether it should really return a WARNING state here.  If so,
> the reason for the WARNING should certainly be mentioned in the plugin
> output.  For the moment, I committed patch which at least adds a line
> explaining the WARNING to the verbose output.

With your patch and mine, I get a bunch of "warning: unable to read
server jitter response." message (I guess one per valid time source on
the remote end) instead of segfault. What's very strange is that on your
server in Berlin this doesn't happen all the time. It never happens on
my localhost and always does on a Sun (Solaris 10) box I just set up
(solaris.beaubien.net, you can try if you like).

I'm under the impression that the problem is related to what is sent to
the server, as either all or no request work. I'm gonna have a deeper
look now...

> The memcpy(3) following the realloc(3) call writes out of bounds as soon
> as peer_offset is >0.  I committed the following patch:
> 
> ---------- 8< ----------------------------------------------------------
> --- check_ntp.c 31 Mar 2007 17:35:08 -0000      1.17
> +++ check_ntp.c 31 Mar 2007 18:40:46 -0000
> @@ -506,6 +506,7 @@
>         ntp_control_message req;
>         double rval = 0.0, jitter = -1.0;
>         char *startofvalue=NULL, *nptr=NULL;
> +       void *tmp;
> 
>         /* Long-winded explanation:
>          * Getting the jitter requires a number of steps:
> @@ -539,8 +540,10 @@
>                  * we represent as a ntp_assoc_status_pair datatype.
>                  */
>                 npeers+=(ntohs(req.count)/sizeof(ntp_assoc_status_pair));
> -               peers=(ntp_assoc_status_pair*)realloc(peers, sizeof(ntp_assoc_status_pair)*npeers);
> -               memcpy((void*)((ptrdiff_t)peers+peer_offset), (void*)req.data, sizeof(ntp_assoc_status_pair)*npeers);
> +               if((tmp=realloc(peers, sizeof(ntp_assoc_status_pair)*npeers)) == NULL)
> +                       free(peers), die(STATE_UNKNOWN, "can not (re)allocate 'peers' buffer\n");
> +               peers=tmp;
> +               memcpy((void*)((ptrdiff_t)peers+peer_offset), (void*)req.data, ntohs(req.count));
>                 peer_offset+=ntohs(req.count);
>         } while(req.op&REM_MORE);
> ---------- 8< ----------------------------------------------------------
> 
> Could you test whether this fixes the problem for you?

Well, strangely I couldn't reproduce the problem. What's even stranger
is that I haven't recompiled anything since then. Thanks for fixing
whatever was wrong there though :)

Thomas