4

I was recently writing code for a custom serial communication protocol. What I did was, I used a part(8/16 bit) of the receiving data to denote how big the frame size is. Based on this data I expect that no of data to follow. I use Crc to accept or reject a frame. But I won't be able to include the frame length data in the Crc, since on the receiving end I should know how much data to expect, before processing a frame.

The issue that I faced is, occasionally this frame length data gets corrupted and it fools the receiver into receiving that many bytes, whereas the receiving array size is much lesser than that. This corrupts a lot of critical system variables that is present in the consecutive memory locations.

How do I prevent the buffer overflow from happening? My thoughts on this 1) Reject the framelength data if it goes beyond a certain value. 2) use a datatype that limits the max no. Like using a short which limits the scope of the array index to 256 memory locations, and create a buffer with 280 bytes. 3) allocate memory in a separate location, so that it doesn't affect the critical system variables.

One thing I used to prevent getting stuck in the receiving loop is by using a timeout. But I overlooked this aspect of the issue. It look me lot if time to confirm and reproduce the issue, since thus code is part of a larger system code, and I'm not an expert here.

Generally how to safely handle this type of issues?

Also: what are general considerations or standard practices to follow when using an array, to prevent it from overflowing?

seetharaman
  • 141
  • 4
  • Pardon me if I misused some terminologies. – seetharaman Apr 09 '16 at 18:44
  • I'd say 2) is the right answer. (Although `short` is 2 bytes, and what you want is a one byte value.) – user3386109 Apr 09 '16 at 18:57
  • I don't understand how the frame length data getting corrupted could fool the receiver into a buffer overrun. Surely the receiver reads exactly the number of bytes it is told to expect (or attempts to do so), and it is within the receiver's power to ensure that it has a sufficient buffer for that, even if the received frame size is wrong. – John Bollinger Apr 09 '16 at 19:00
  • 2
    Send the buffer length twice (perhaps inverted the second time). If it matches, it's OK. – Weather Vane Apr 09 '16 at 19:03
  • 1
    You can improve error detection by choosing a serial protocol that includes parity bits. – John Bollinger Apr 09 '16 at 19:04
  • @JohnBollinger I receive the frame length data and keep receiving data until it has received data bytes that equals the frame length data. – seetharaman Apr 09 '16 at 19:06
  • @user3386109 yes, but my question is, is this the best way to do it? – seetharaman Apr 09 '16 at 19:08
  • @WeatherVane yes I could try that.. FEC(forward error correction) – seetharaman Apr 09 '16 at 19:10
  • @JohnBollinger pairity could detect only one bit errors. Probably what weatherVane said could work. – seetharaman Apr 09 '16 at 19:12
  • 3
    That's the simple solution. The other solution is to send a fixed-sized header that itself has a CRC, or some sort of forward-error-correction. – user3386109 Apr 09 '16 at 19:12
  • 3
    I'm saying that even if the client receives a corrupted frame length, there is no reason why that should be able to fool the client into a buffer overrun. The client knows how big its buffer is, and it has complete control over how many bytes it stores where. – John Bollinger Apr 09 '16 at 19:18
  • @JohnBollinger yes, very true, I just overlooked it the first time. Now thinking how to make it better, yet keeping the code safe. – seetharaman Apr 09 '16 at 19:22
  • If this is an issue and you are getting frequent data corruption, do the parity check too. Belt and braces. – Weather Vane Apr 09 '16 at 19:22
  • 2
    There is in any case no 100% foolproof mechanism for error detection. It's all a question of much overhead -- both data-wise and computation-wise -- you want to invest into error detection, and how improbable you want it to be that the data are corrupted in a way that cannot be detected. – John Bollinger Apr 09 '16 at 19:23
  • If the length is corrupted, you have no idea what else may have been corrupted, so you should probably discard the entire frame if its length exceeds that expected. If it is only the length that is ever corrupted, you should perhaps fix that problem first, because that sounds like an software error rather than environmental. Mechanisms for error detection should be for exceptional cases (external interference) rather then to solve a poorly designed or implemented connection that is intrinsically unreliable - for that you need a more sophisticated error detection/correction system. – Clifford Apr 10 '16 at 06:42

2 Answers2

1

There's a lot of things that can be used to minimize this issue, however there is no one size fit's all solution for this in general. You have to make decisions and understand what happens if something goes wrong and have your system be able to handle it.

You have to figure out what exactly fits best for your system. For example if you never expect a message to be greater than 256 then declaring the size of your buffer as 0xFF and the index of your buffer as a uint8_t you'll never be able to exceed it inserting one byte at a time as the index will never reach 256 and overflow back to 0. The downside of that is of course if this does happen, you overwrite some of the data received, but the crc check should reveal error in most cases.

Another thing you can do is compare the data length to the buffer max and not store the message if it exceeds your buffer. So you would outright reject any messages who's data length is too big and received, but not store data coming across. Obviously if the data length comes in corrupted often, you'll have a lot of issues with this.

To be honest, the best method is to rethink your custom serial communication protocol. It doesn't sound like it takes the errors you are encountering very well. You can have sync bytes at the beginning and end of your message to just make sure that you are actually receiving good data and CRC the entire message no problem and define max data packet sizes that will go across the wire and signal how many packets are to be received. If the communication protocol isn't very good then you have to rethink it from the bottom up.

Dom
  • 1,687
  • 6
  • 27
  • 37
  • I'll try using unit8. I already use sync bytes at the start of frame. Only after I detect the sync bytes, I get this frame size data. It seems like data corruption occurs in a random byte(s) in a frame and the other bytes seem to be fine. – seetharaman Apr 13 '16 at 03:25
1

first, check for framing errors, check for parity errors

second, check the magnitude of the data size. if too big or too small, reject the whole data block

user3629249
  • 16,402
  • 1
  • 16
  • 17
  • I'm using a header to detect the start of frame and for bit positioning. As of pairity, I'm not using UART, so calculating in SW will be an overhead. Checking the size of the frame and rejecting may work. – seetharaman Apr 13 '16 at 02:42