Skip to content

Commit

Permalink
HAL_PX4: fixed a buffer handling bug
Browse files Browse the repository at this point in the history
BUF_SPACE() was badly buggy, which could lead to memory corruption
  • Loading branch information
Andrew Tridgell committed Feb 20, 2013
1 parent 160e5fa commit c373429
Showing 1 changed file with 16 additions and 9 deletions.
25 changes: 16 additions & 9 deletions libraries/AP_HAL_PX4/UARTDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <fcntl.h>
#include <termios.h>
#include <drivers/drv_hrt.h>
#include <assert.h>

using namespace PX4;

Expand Down Expand Up @@ -47,7 +48,6 @@ void PX4UARTDriver::begin(uint32_t b, uint16_t rxS, uint16_t txS)
v = fcntl(_fd, F_GETFL, 0);
fcntl(_fd, F_SETFL, v | O_NONBLOCK);

_initialised = true;
if (rxS == 0) {
rxS = 128;
}
Expand All @@ -56,6 +56,9 @@ void PX4UARTDriver::begin(uint32_t b, uint16_t rxS, uint16_t txS)
}
}

_initialised = false;
while (_in_timer) hal.scheduler->delay(1);

if (b != 0) {
// set the baud rate
struct termios t;
Expand All @@ -70,33 +73,31 @@ void PX4UARTDriver::begin(uint32_t b, uint16_t rxS, uint16_t txS)
allocate the read buffer
*/
if (rxS != 0 && rxS != _readbuf_size) {
_initialised = false;
while (_in_timer) hal.scheduler->delay(1);
_readbuf_size = rxS;
if (_readbuf != NULL) {
free(_readbuf);
}
_readbuf = (uint8_t *)malloc(_readbuf_size);
_readbuf_head = 0;
_readbuf_tail = 0;
_initialised = true;
}

/*
allocate the write buffer
*/
if (txS != 0 && txS != _writebuf_size) {
_initialised = false;
while (_in_timer) hal.scheduler->delay(1);
_writebuf_size = txS;
if (_writebuf != NULL) {
free(_writebuf);
}
_writebuf = (uint8_t *)malloc(_writebuf_size);
_writebuf = (uint8_t *)malloc(_writebuf_size+16);
_writebuf_head = 0;
_writebuf_tail = 0;
_initialised = true;
}

if (_writebuf_size != 0 && _readbuf_size != 0) {
_initialised = true;
}
}

void PX4UARTDriver::begin(uint32_t b)
Expand Down Expand Up @@ -191,7 +192,7 @@ void PX4UARTDriver::_vprintf(const char *fmt, va_list ap)
buffer handling macros
*/
#define BUF_AVAILABLE(buf) ((buf##_head > (_tail=buf##_tail))? (buf##_size - buf##_head) + _tail: _tail - buf##_head)
#define BUF_SPACE(buf) (((_head=buf##_head) > buf##_tail)?(buf##_tail - _head) - 1:((buf##_size - buf##_tail) + _head) - 1)
#define BUF_SPACE(buf) (((_head=buf##_head) > buf##_tail)?(_head - buf##_tail) - 1:((buf##_size - buf##_tail) + _head) - 1)
#define BUF_EMPTY(buf) (buf##_head == buf##_tail)
#define BUF_ADVANCETAIL(buf, n) buf##_tail = (buf##_tail + n) % buf##_size
#define BUF_ADVANCEHEAD(buf, n) buf##_head = (buf##_head + n) % buf##_size
Expand Down Expand Up @@ -284,6 +285,7 @@ size_t PX4UARTDriver::write(const uint8_t *buffer, size_t size)
}
if (_writebuf_tail < _head) {
// perform as single memcpy
assert(_writebuf_tail+size <= _writebuf_size);
memcpy(&_writebuf[_writebuf_tail], buffer, size);
BUF_ADVANCETAIL(_writebuf, size);
return size;
Expand All @@ -292,11 +294,13 @@ size_t PX4UARTDriver::write(const uint8_t *buffer, size_t size)
// perform as two memcpy calls
uint16_t n = _writebuf_size - _writebuf_tail;
if (n > size) n = size;
assert(_writebuf_tail+n <= _writebuf_size);
memcpy(&_writebuf[_writebuf_tail], buffer, n);
BUF_ADVANCETAIL(_writebuf, n);
buffer += n;
n = size - n;
if (n > 0) {
assert(_writebuf_tail+n <= _writebuf_size);
memcpy(&_writebuf[_writebuf_tail], buffer, n);
BUF_ADVANCETAIL(_writebuf, n);
}
Expand Down Expand Up @@ -380,17 +384,20 @@ void PX4UARTDriver::_timer_tick(void)
perf_begin(_perf_uart);
if (_readbuf_tail < _head) {
// one read will do
assert(_readbuf_tail+n <= _readbuf_size);
int ret = ::read(_fd, &_readbuf[_readbuf_tail], n);
if (ret > 0) {
BUF_ADVANCETAIL(_readbuf, ret);
}
} else {
uint16_t n1 = _readbuf_size - _readbuf_tail;
assert(_readbuf_tail+n1 <= _readbuf_size);
int ret = ::read(_fd, &_readbuf[_readbuf_tail], n1);
if (ret > 0) {
BUF_ADVANCETAIL(_readbuf, ret);
}
if (ret == n1 && n != n1) {
assert(_readbuf_tail+(n-n1) <= _readbuf_size);
ret = ::read(_fd, &_readbuf[_readbuf_tail], n - n1);
if (ret > 0) {
BUF_ADVANCETAIL(_readbuf, ret);
Expand Down

0 comments on commit c373429

Please sign in to comment.