Malformed NMEA can crash XCSoar

henrik1g commented 2 years ago

XCSoar versions having and not having the problem

v7.25 and v7.24 both affected

Expected behavior

When malformed NMEA is received due to various reasons (for example
https://github.com/XCSoar/XCSoar/issues/1031 ) XCS should discard the malformed sentence without crashing.

Actual behavior

When receiving malformed NMEA XCSoar eventually crashes without any error message or hint in the logs what went wrong.

Steps to reproduce the behavior

The crash comes faster the more corrupted the NMEA is. Sometimes instantly, somtimes after a few seconds. Maximum a few minutes. The issue seems to be independent of the driver. FLARM, Generic, and Volkslogger were all affected when sending

$GPRMC,143618.00,A,2944.49207,N,09528.00179,W,0.235,357.14,300822,,,A*74
$PGRMZ,121,F,2*38
$GPGGA,143618.00,2944.49207,N,09528.00179,W,1,05,6.95,64.4,M,-23.7,M,,*5F
$PFLAU,0,1,1,1,0,,0,,*63

once each second through corrupted connection. (See https://github.com/XCSoar/XCSoar/issues/1031 )

Switching on port debug in XCSoar device dialog did not help to get more info since it seems normal RX-Thread received sentences are not logged, only two way transactions. There was no useful info in xcsoar.log and no file in XCSoarData/crash

Do you have any idea what may have caused this?

Maybe the handling of incoming NMEA is not error tolerant enough.

Do you have an idea how to solve the issue?

If you point me to how I can do real debugging on the android device I can investigate further which type of malformed data causes the crash exactly. But so far I can build xcsoar for android, but I have no idea yet how to do debugging on the device.

MaxKellermann commented 2 years ago

I can't reproduce the crash with your data, and you did not post any information about the crash.

henrik1g commented 2 years ago

I can't reproduce the crash with your data, and you did not post any information about the crash.

The NMEA data needs to be corrupted to produce the crash. But I don't know yet in which way exactly since when xcsoar crashes I have no information about the last processed sentence. Please point me in the right direction how to do debugging on android. I will then investigate further and provide more info.

MaxKellermann commented 2 years ago

Sorry, I never debug on Android. But if it "crashes" on Android, there must be information about the crash in Android's system log (logcat). If there is nothing, then XCSoar did not crash and this bug report is bogus. Check again.

henrik1g commented 2 years ago

I did not check logcat yet, only xcsoar.log. I will do so and attach the logs later. Have to work now ;-)

henrik1g commented 2 years ago

So I got some valuable information from logcat:

[09-01 21:55:00.382 10368:10503 E/AndroidRuntime]
FATAL EXCEPTION: Thread-4
Process: org.xcsoar, PID: 10368
java.lang.ArrayIndexOutOfBoundsException: src.length=65 srcPos=2 dst.length=61 dstPos=0 length=62
    at java.lang.System.arraycopy(Native Method)
    at com.felhr.usbserial.FTDISerialDevice.copyData(FTDISerialDevice.java:527)
    at com.felhr.usbserial.FTDISerialDevice.adaptArray(FTDISerialDevice.java:508)
    at com.felhr.usbserial.UsbSerialDevice$WorkerThread.doRun(UsbSerialDevice.java:359)
    at com.felhr.usbserial.AbstractWorkerThread.run(AbstractWorkerThread.java:21)

[09-01 21:55:00.389 2855:9713 W/ActivityManager]
  Force finishing activity org.xcsoar/.XCSoar

So it seems the malformed NMEA, that shows up in the XCSoar serial monitor is not the cause of the crash, but rather the serial stream corruption AND the crash originate from the low level code in felhr.usbserial. Maybe just two symptoms of the same problem. So I suggest to close this bug report, but at the same time this raises the importance of https://github.com/XCSoar/XCSoar/issues/1031 since the code in fehl.usbserial does not just corrupt NMEA at certain baudrates, it can even crash XCSoar.

MaxKellermann commented 2 years ago

So I suggest to close this bug report, but at the same time this raises the importance of #1031

That's not a good suggestion, because your log clearly proves the presence of this crash bug (but it's not about "malformed NMEA", but an internal crash of the UsbSerial library which XCSoar uses). Once this UsbSerial bug is fixed, maybe #1031 is fixed as well, but we so far we don't know anything about #1031 yet. We have no angle to attack #1031.

MaxKellermann commented 2 years ago

PR for upstream UsbSerial: https://github.com/felHR85/UsbSerial/pull/356

XCSoar / XCSoar