Comment 19 for bug 1481388

Revision history for this message
In , Dave Hart (hart-ntp) wrote :

Thanks for the report. The additional debugging paste is short enough to include directly in the comments:

==24767== Invalid read of size 8
==24767== at 0x411048: input_handler (ntp_io.c:3621)
==24767== by 0x414B84: ntpdmain (ntpd.c:1078)
==24767== by 0x406448: main (ntpd.c:356)
==24767== Address 0x5e897f0 is 0 bytes inside a block of size 32 free'd
==24767== at 0x4C26649: free (in /lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24767== by 0x411072: input_handler (ntp_io.c:3619)
==24767== by 0x414B84: ntpdmain (ntpd.c:1078)
==24767== by 0x406448: main (ntpd.c:356)

The code in question is:

#ifdef HAS_ROUTING_SOCKET
 /*
  * scan list of asyncio readers - currently only used for routing sockets
  */
 asyncio_reader = asyncio_reader_list;

 while (asyncio_reader != NULL) {
  if (FD_ISSET(asyncio_reader->fd, &fds)) {
   ++select_count;
   (asyncio_reader->receiver)(asyncio_reader); /*3619 */
  }
  asyncio_reader = asyncio_reader->link; /* 3621 */
 }
#endif /* HAS_ROUTING_SOCKET */

line 3619 is calling process_routing_msgs() which, after root is dropped, is noticing a failed read or other error and removing the entry from asyncio_reader_list and free()ing it, triggering the valgrind catch.

I bet can be worked around by adding -U 0 to the command line to disable dynamic interface updates, I suspect (I could be wrong, too). To patch it, we need to add a "next_asyncio_reader" local variable of the same type as asyncio_reader, and assign to it asyncio_reader->link before if (FD_ISSET(..., and change the asyncio_reader assignment to use the saved next_asyncio_reader. I will get that ready for ntp-dev, and am requesting 4.2.6 blocking in case we do another release of that stable version.