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

gpsd bugfixes



Hi Release Team,

today I received #603439 on gpsd, reporting a segfault while handling AIS data
in gpsd. It makes gpsd more or less useless for ship data tracking as it will
crash at random times. Fixing this bug is easy as the bug reporter (who is well
known for submitting good patches to gpsd) delivered a patch with the bug report.

Also we (as in gpsd upstream) collected several other bugfixes during the last
weeks. The bugs are not as bad as the one mentioned above, but it would still
make sense to fix them in Squeeze imho. As gpsd runs a long regression test
suite at build time it would be easy for me to see if they introduce issues.

What I'd like you to decide on now is if the next upload should fix all the
easily fixable issues or only the serious AIS related one.
Attached are patches 0009-0014, with 0014 being the fix for the serious issue.
So please let me know if you'd prefer an upload with 0009-0014 or a fix with
0014 only.

Thanks and cheers,

Bernd

-- 
 Bernd Zeimetz                            Debian GNU/Linux Developer
 http://bzed.de                                http://www.debian.org
 GPG Fingerprint: ECA1 E3F2 8E11 2432 D485  DD95 EB36 171A 6FF9 435F
From: Jon Schlueter <jon.schlueter@gmail.com>
Date: Wed, 6 Oct 2010 16:32:09 -0400
Subject: [PATCH] gpsutil.c: fill_dop bug if sats < 4

bug present where it would try to invert matrix with out enough data...
added guard to prevent trying to calculate dop's if we don't have 4 sats
with current code.
---
 gpsutils.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/gpsutils.c b/gpsutils.c
index 6b94de0..1bf200e 100644
--- a/gpsutils.c
+++ b/gpsutils.c
@@ -466,6 +466,8 @@ gps_mask_t fill_dop(const struct gps_data_t * gpsdata, struct dop_t * dop)
     double xdop, ydop, hdop, vdop, pdop, tdop, gdop;
     int i, j, k, n;
 
+    memset(satpos, 0, sizeof(satpos));
+
 #ifdef __UNUSED__
     gpsd_report(LOG_INF, "Satellite picture:\n");
     for (k = 0; k < MAXCHANNELS; k++) {
@@ -488,6 +490,16 @@ gps_mask_t fill_dop(const struct gps_data_t * gpsdata, struct dop_t * dop)
 	n++;
     }
 
+    /* If we don't have 4 satellites then we don't have enough information to calculate DOPS */
+    if (n < 4) {
+	gpsd_report(LOG_DATA + 2, "Not enough Satellites available %d < 4:\n",
+		    n);
+	return 0;		/* Is this correct return code here? or should it be ERROR_IS */
+    }
+
+    memset(prod, 0, sizeof(prod));
+    memset(inv, 0, sizeof(inv));
+
 #ifdef __UNUSED__
     gpsd_report(LOG_INF, "Line-of-sight matrix:\n");
     for (k = 0; k < n; k++) {
-- 
From: Gary E. Miller <gem@rellim.com>
Date: Tue, 26 Oct 2010 15:42:57 -0700
Subject: [PATCH] GPZDA reported broken by aquarat, and others.  Been broken since before
 git.  nmea_parse() was expecting 7 fields when their are only 6.  Since
 we only use the first 4 fields that is the new minimum.

This seems to break one regression test, but appears to be a valid fix.
---
 driver_nmea.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/driver_nmea.c b/driver_nmea.c
index fbefc66..cc001a8 100644
--- a/driver_nmea.c
+++ b/driver_nmea.c
@@ -931,7 +931,7 @@ gps_mask_t nmea_parse(char *sentence, struct gps_device_t * session)
 	"GSV", 0, processGPGSV}, {
 	"VTG", 0, NULL},	/* ignore Velocity Track made Good */
 	{
-	"ZDA", 7, processGPZDA}, {
+	"ZDA", 4, processGPZDA}, {
 	"GBS", 7, processGPGBS},
 #ifdef TNT_ENABLE
 	{
-- 
From: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Date: Sun, 5 Sep 2010 13:42:26 -0400
Subject: [PATCH] Bug gpsd:17494 fix: cgps don't exit if interrupted

Do not exit if select is interrupted by a signal. Since we have signal handlers for INT and HUP they still work but WINCH does not. Closes gpsd:17494.
---
 cgps.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/cgps.c b/cgps.c
index 36abc3c..e357836 100644
--- a/cgps.c
+++ b/cgps.c
@@ -895,6 +895,9 @@ int main(int argc, char *argv[])
 	data = select(gpsdata.gps_fd + 1, &rfds, NULL, NULL, &timeout);
 
 	if (data == -1) {
+            if (errno == EINTR) {
+                continue;
+            }
 	    fprintf(stderr, "cgps: socket error 3\n");
 	    exit(2);
 	} else if (data) {
-- 
From: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Date: Sat, 6 Nov 2010 11:55:31 -0400
Subject: [PATCH] Bug #17637: /dev/ttySAC1 is always opened in read-only mode

More info:
1) /dev/ttySAC1 is a serial port used on samsung s3c2440 (e.g. openmoko
	freerunner):

$ stat /dev/ttySAC1
File: `/dev/ttySAC1'
Size: 0 Blocks: 0 IO Block: 4096 character special file
Device: ch/12d Inode: 161 Links: 1 Device type: cc,41
Access: (0660/crw-rw----) Uid: ( 0/ root) Gid: ( 20/ dialout)
Access: 2010-10-19 01:23:13.893489000 +0300
Modify: 2010-10-18 20:21:22.089934000 +0300
Change: 2010-10-18 20:21:22.089934000 +0300

2) I think the problem is in gpsd_classify(). It assumes that
	all serial ports have major number of 4. It even links to
	ftp://ftp.kernel.org/pub/linux/docs/device-list/devices-2.6+.txt
	as a reference. However, that file nowadays lists also

204 char Low-density serial ports
0 = /dev/ttyLU0 LinkUp Systems L72xx UART - port 0
1 = /dev/ttyLU1 LinkUp Systems L72xx UART - port 1
2 = /dev/ttyLU2 LinkUp Systems L72xx UART - port 2
3 = /dev/ttyLU3 LinkUp Systems L72xx UART - port 3
4 = /dev/ttyFB0 Intel Footbridge (ARM)
5 = /dev/ttySA0 StrongARM builtin serial port 0
6 = /dev/ttySA1 StrongARM builtin serial port 1
...

The patch adds 204 to the list of recognized major numbers.

Signed-off-by: Jon Schlueter <jon.schlueter@gmail.com>
---
 serial.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/serial.c b/serial.c
index ec37836..28c6bef 100644
--- a/serial.c
+++ b/serial.c
@@ -59,7 +59,7 @@ static sourcetype_t gpsd_classify(const char *path)
 	 * ftp://ftp.kernel.org/pub/linux/docs/device-list/devices-2.6+.txt
 	 */
 	int devmajor = major(sb.st_rdev);
-	if (devmajor == 4)
+	if (devmajor == 4 || devmajor == 204)
 	    devtype = source_rs232;
 	else if (devmajor == 188)
 	    devtype = source_usb;
-- 
From: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Date: Sat, 6 Nov 2010 12:08:34 -0400
Subject: [PATCH] Bug #17638: control socket & command can return OK even if write fails

First patch to fix the ! Command

On of the symptoms of the bug 017637 are that writes to the serial port
fail. In particular

echo "&/dev/ttySAC1=01" | socat - UNIX-CONNECT:gpsd.sock

causes gpsd to do

write(7, "\1", 1) = -1 EBADF (Bad file descriptor)
write(8, "OK\n", 3)

which is wrong in two different ways:

1) It should not attempt to write to a read-only device
2) it should not print "OK" when the write fails

The attached patch tries to fix both issues for the ! command. If you
agree, I'll write a second patch that does the same treatment for the &
command.

Signed-off-by: Jon Schlueter <jon.schlueter@gmail.com>
---
 gpsd.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/gpsd.c b/gpsd.c
index 298a183..9268a18 100644
--- a/gpsd.c
+++ b/gpsd.c
@@ -821,10 +821,21 @@ static void handle_control(int sfd, char *buf)
 	} else {
 	    *eq++ = '\0';
 	    if ((devp = find_device(stash))) {
-		gpsd_report(LOG_INF, "<= control(%d): writing to %s \n", sfd,
-			    stash);
-		ignore_return(write(devp->gpsdata.gps_fd, eq, strlen(eq)));
-		ignore_return(write(sfd, "OK\n", 3));
+		if (devp->context->readonly || (devp->sourcetype <= source_blockdev)) {
+		    gpsd_report(LOG_WARN, "<= control(%d): attempted to write to a read-only device\n",
+				sfd);
+		    ignore_return(write(sfd, "ERROR\n", 6));
+		} else {
+		    gpsd_report(LOG_INF, "<= control(%d): writing to %s \n", sfd,
+				stash);
+		    if (write(devp->gpsdata.gps_fd, eq, strlen(eq)) <= 0) {
+			gpsd_report(LOG_WARN, "<= control(%d): write to device failed\n",
+				    sfd);
+			ignore_return(write(sfd, "ERROR\n", 6));
+		    } else {
+			ignore_return(write(sfd, "OK\n", 3));
+		    }
+		}
 	    } else {
 		gpsd_report(LOG_INF, "<= control(%d): %s not active \n", sfd,
 			    stash);
-- 
From: =?UTF-8?q?Nirgal=20Vourg=C3=A8re?= <jmv_deb@nirgal.com>
Date: Sun, 14 Nov 2010 02:27:35 +0000
Subject: [PATCH] Fixes SEGV on reception of short aivdm type 26 message

gpsd is crashing when receiving some messages such as
!AIVDM,1,1,,A,J=IJuwOmoTt,2*3F
because unsigned value ais->type26.bitcount is sometimes assigned
a negative value.

Signed-off-by: Bernd Zeimetz <bernd@bzed.de>
---
 driver_aivdm.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/driver_aivdm.c b/driver_aivdm.c
index af0c890..2f2138f 100644
--- a/driver_aivdm.c
+++ b/driver_aivdm.c
@@ -802,6 +802,10 @@ bool aivdm_decode(const char *buf, size_t buflen,
 	    }
 	    ais->type26.addressed	= (bool)UBITS(38, 1);
 	    ais->type26.structured	= (bool)UBITS(39, 1);
+	    if (ais_context->bitlen < 40 + 16*ais->type26.structured + 30*ais->type26.addressed + 20) {
+		gpsd_report(LOG_WARN, "AIVDM message type 26 too short for mode.\n");
+		return false;
+	    }
 	    if (ais->type26.addressed)
 		ais->type26.dest_mmsi   = UBITS(40, 30);
 	    if (ais->type26.structured)
-- 

Reply to: