arduino-libraries / ArduinoBLE

ArduinoBLE library for Arduino
GNU Lesser General Public License v2.1
310 stars 205 forks source link

HCICardioTransport - RingBuffer _rxBuf may not be safe #318

Open KurtE opened 1 year ago

KurtE commented 1 year ago

I was trying to build one of the ArduinoBLE examples (scan), using my updated core code for UNOR4 Wifi, and: HCIVirtualTransport.cpp failed to compile as RingBufferN was not defined... This was with my current version I removed the usage of it from the Serial code. Fix for this one is to explicitly include it in the file: i.e. to add the line: #include "api/RingBuffer.h"

While I was looking at how the RingBuffer code was used in this library, I believe that the usage in the HCICardioTransport class may be unsafe.

That is the buf.read_char is called from its main read method.

int HCICordioTransportClass::read()
{
  return _rxBuf.read_char();
}

And the store_char is called in the method HandleRxData which is called by the method onDataReceived, which appears to be called by an event.

As I mentioned in the issue: https://github.com/arduino/ArduinoCore-API/issues/195 Both the read_char and store_char both modify a member variable: _numElems To either increment or decrement it.

The problem is there is a timing window in the current code, Where if the callback happens while in the read_char, right after it reads in the current value of _numElems and before it stores the updated count back out, the callback happens, which updates the count. When it returns, the storing out of the updated count in the read_char will wipeout the count changes that happened in the store_char call(s) - could be be multiple bytes that are added during the callback.

Hope this was clear.