G4lile0 / tinyGS

📡 Open Ground Station Network 🛰
GNU General Public License v3.0
924 stars 177 forks source link

Receiving weird packets, probably memory corruption #67

Closed ReDetection closed 3 years ago

ReDetection commented 3 years ago
Hello. I am pretty confident tinyGS outputs packets which it did not received. During satellite pass in my area I receive a lot of short packets in both serial and web console. At first I thought it is a problem on the satellite side, or maybe I received malformed packet, but now it's clear to me: these are not received via LoRa, it is originated from the ESP chip itself. Some frequent examples: hex string comment
343333204d687a205454474f204c6f5241 433 Mhz TTGO LoRA At first I only saw this and thought it could originate from the satellite
03000000bcb01540641dfb3f54bc423f78 ���¼°@dû?T¼B?x Rubbish can probably be picked out of noise
74696e7967732f3236363437353035312f tinygs/266475051/ Well that's clearly not noise and satellite surely doesn't know these!

What could be the reason? To my experience, this only happens if I use a buffer but do not clear it, or do no length check, or write to a wrong memory location, etc. All the cases could easily lead to undefined behaviour. I'm going to put some check in the code here and there to verify & debug this, Radio.cpp looks particularly interesting to me. Do you have any suggestions?

ReDetection commented 3 years ago

Forgot to mention: I'm using 443 MHz TTGO board v2. Firmware is compiled from source, I tried both tagged 21021701, current master and dev branches.

ReDetection commented 3 years ago

I enabled RADIOLIB_VERBOSE and it looks like packets never actually arrived. I tried to receive test packet and real one – here is the difference:

Test packet:
R   1   8D  
R   13  C   
R   1   8D  
R   1   8D  
W   1   89  
R   1   89  
R   1C  40  
R   12  50  
R   0   54  69  6E  79  47  53  2D  74  65  73  74  20  
R   1   89  
R   13  C   
R   1   89  
W   12  FF  
R   1   89  
R   1A  70  
R   1   89  
R   19  1C  
R   1   89  
R   19  1C  
R   1   89  
R   28  F   
R   29  F0  
R   2A  28  
09:51:57 Packet (12 bytes):
09:51:57 54696e7947532d7465737420
09:51:57 [SX12x8] RSSI:     -52.000000 dBm
[SX12x8] SNR:       7.000000 dB
[SX12x8] Frequency error:   -531.628052 Hz
"Real" one:
R   1   8D  
R   13  1A  
R   1   8D  
R   1   8D  
W   1   89  
R   1   89  
R   1C  0   
R   1   89  
W   12  FF  
R   1   89  
R   1A  3B  
R   1   89  
R   19  F6  
R   1   89  
R   19  F6  
R   1   89  
R   28  0   
R   29  1D  
R   2A  A8  
12:57:03 Packet (26 bytes):
12:57:03 343333204d687a205454474f204c6f5241203332207632000000
12:57:03 [SX12x8] RSSI:     -107.500000 dBm
[SX12x8] SNR:       -2.500000 dB
[SX12x8] Frequency error:   1990.197266 Hz

So, I do not see actual SPI transmission when receiving these broken packets.

ReDetection commented 3 years ago

OK this comes from memory reuse – this buffer stays like this before reading the data from the transceiver. I wonder if it's better to silence all error packets or clear the buffer

4m1g0 commented 3 years ago

Hi, thanks for the report.

This might partially fix it: 890006d446ff7b987571222baa5a2823c6d5efaf

It will be merged into the next release. I'll leave the issue open until we see if it works.

ReDetection commented 3 years ago

@4m1g0 thanks for the response. Yeah, I tried this approach by for couple of days and it proved to work fine. One thing I did different: I still check for respLen > 0 because I wasn't sure if it's safe to allocate and use zero-sized array :)

ReDetection commented 3 years ago

I consider it done, thanks.

4m1g0 commented 3 years ago

Thanks!