Address Savannah bug #46648: gpsd crashes and buffer overflow is reported...

...when terminated.

Perttu Salmela writes:

gpsd is started and terminated:

gpsd:INFO: launching (Version 3.15~dev)
gpsd:INFO: listening on port gpsd
gpsd:PROG: NTP: shmat(0,0,0) succeeded, segment 0
gpsd:PROG: NTP: shmat(32769,0,0) succeeded, segment 1
gpsd:PROG: NTP: shmat(65538,0,0) succeeded, segment 2
gpsd:PROG: NTP: shmat(98307,0,0) succeeded, segment 3
gpsd:PROG: NTP: shmat(131076,0,0) succeeded, segment 4
gpsd:PROG: NTP: shmat(163845,0,0) succeeded, segment 5
gpsd:PROG: NTP: shmat(196614,0,0) succeeded, segment 6
gpsd:PROG: NTP: shmat(229383,0,0) succeeded, segment 7
gpsd:PROG: successfully connected to the DBUS system bus
gpsd:PROG: shmget(0x47505344, 8928, 0666) for SHM export succeeded
gpsd:PROG: shmat() for SHM export succeeded, segment 262152
gpsd:INFO: stashing device /dev/ttymxc2 at slot 0
gpsd:INFO: running with effective group ID 0
gpsd:INFO: running with effective user ID 0
gpsd:INFO: startup at 2015-10-31T11:04:55.000Z (1446289495)
^C*** buffer overflow detected ***: ./gpsd terminated
Aborted (core dumped)

This does not happen when gpsd is started with '-n' no-wait option. If
started with '-n' device is opened fine and gpsd is terminated fine.

The problem seems to be that function gpsd.c:gps_add_device sets
devp->gpsdata.gps_fd = UNALLOCATED_FD (=-1) if no-wait ('-n') flag is
not set. Next, in the main function around line 2166:

case AWAIT_NOT_READY:
  for (device = devices; device < devices + MAX_DEVICES; device++)
     if (allocated_device(device) && FD_ISSET(device->gpsdata.gps_fd, &efds)) {

FD_ISSET macro is called with invalid FD (-1). Adding the FD validity
check before FD_ISSET fixes the crash:

if (allocated_device(device) &&
(0 <= device->gpsdata.gps_fd && device->gpsdata.gps_fd < FD_SETSIZE) &&
FD_ISSET(device->gpsdata.gps_fd, &efds)) {

It is still a bit unclear for me should free_device(device) be called
even if FD is invalid.

The issue occurs on embedded arm platform and may depend on
implementation of FD_ISSET macro. The man page says "POSIX requires fd
to be a valid file descriptor". I can see that FD_ISSET is called in
couple of places elsewhere and FD validity is checked there.

The issue does not happen if client is connected. E.g. if gpspipe is
run and thereafter gpsd is terminated. This is expected since in such
case the FD must be valid as gpsd connects to device.

Output with proposed fix is:
gpsd:INFO: launching (Version 3.15~dev)
gpsd:INFO: listening on port gpsd
gpsd:PROG: NTP: shmat(0,0,0) succeeded, segment 0
gpsd:PROG: NTP: shmat(32769,0,0) succeeded, segment 1
gpsd:PROG: NTP: shmat(65538,0,0) succeeded, segment 2
gpsd:PROG: NTP: shmat(98307,0,0) succeeded, segment 3
gpsd:PROG: NTP: shmat(131076,0,0) succeeded, segment 4
gpsd:PROG: NTP: shmat(163845,0,0) succeeded, segment 5
gpsd:PROG: NTP: shmat(196614,0,0) succeeded, segment 6
gpsd:PROG: NTP: shmat(229383,0,0) succeeded, segment 7
gpsd:PROG: successfully connected to the DBUS system bus
gpsd:PROG: shmget(0x47505344, 8928, 0666) for SHM export succeeded
gpsd:PROG: shmat() for SHM export succeeded, segment 262152
gpsd:INFO: stashing device /dev/ttymxc2 at slot 0
gpsd:INFO: running with effective group ID 0
gpsd:INFO: running with effective user ID 0
gpsd:INFO: startup at 2015-10-31T11:31:19.000Z (1446291079)
^Cgpsd:WARN: received terminating signal 2.
gpsd:WARN: exiting.

1 files changed, 8 insertions, 1 deletions

diff --git a/gpsd.c b/gpsd.c
index 416084cd..bba938f1 100644
--- a/gpsd.c
+++ b/gpsd.c
@@ -2171,7 +2171,14 @@ int main(int argc, char *argv[])
 	    break;
 	case AWAIT_NOT_READY:
 	    for (device = devices; device < devices + MAX_DEVICES; device++)
-		if (allocated_device(device) && FD_ISSET(device->gpsdata.gps_fd, &efds)) {
+		/*
+		 * The file descriptor validity check is reqiured on some ARM 
+		 * platforms to prevent a core dump.  This may be due to an
+		 * implimentation error in FD_ISSET().
+		 */
+		if (allocated_device(device)
+		    && (0 <= device->gpsdata.gps_fd && device->gpsdata.gps_fd < FD_SETSIZE)
+		    && FD_ISSET(device->gpsdata.gps_fd, &efds)) {
 		    deactivate_device(device);
 		    free_device(device);
 		}