[posix-app] fix bugs of posix apps (#2888)

1. avoid possibility of reentrant to Hdlc::Decode
2. do not send unwanted ACK to core stack
This commit is contained in:
Yakun Xu
2018-07-13 10:50:14 -05:00
committed by Jonathan Hui
parent 51a1a5f12b
commit ae4e5171ef
5 changed files with 58 additions and 63 deletions
+5 -2
View File
@@ -561,6 +561,7 @@ bool otPlatRadioGetPromiscuous(otInstance *aInstance)
void radioReceive(otInstance *aInstance)
{
bool isAck;
ssize_t rval = recvfrom(sSockFd, (char *)&sReceiveMessage, sizeof(sReceiveMessage), 0, NULL, NULL);
if (rval < 0)
@@ -581,7 +582,9 @@ void radioReceive(otInstance *aInstance)
sReceiveFrame.mLength = (uint8_t)(rval - 1);
if (sAckWait && sTransmitFrame.mChannel == sReceiveMessage.mChannel && isFrameTypeAck(sReceiveFrame.mPsdu) &&
isAck = isFrameTypeAck(sReceiveFrame.mPsdu);
if (sAckWait && sTransmitFrame.mChannel == sReceiveMessage.mChannel && isAck &&
getDsn(sReceiveFrame.mPsdu) == getDsn(sTransmitFrame.mPsdu))
{
sState = OT_RADIO_STATE_RECEIVE;
@@ -590,7 +593,7 @@ void radioReceive(otInstance *aInstance)
otPlatRadioTxDone(aInstance, &sTransmitFrame, &sReceiveFrame, OT_ERROR_NONE);
}
else if ((sState == OT_RADIO_STATE_RECEIVE || sState == OT_RADIO_STATE_TRANSMIT) &&
(sReceiveFrame.mChannel == sReceiveMessage.mChannel))
(sReceiveFrame.mChannel == sReceiveMessage.mChannel) && (!isAck || sPromiscuous))
{
radioProcessFrame(aInstance);
}
+8 -16
View File
@@ -38,7 +38,7 @@
#include <assert.h>
#include <string.h>
#include "code_utils.h"
#include <common/code_utils.hpp>
namespace ot {
@@ -48,16 +48,16 @@ otError FrameQueue::Push(const uint8_t *aFrame, uint8_t aLength)
uint16_t newTail = mTail + aLength + 1;
assert(aFrame != NULL);
otEXPECT_ACTION(aFrame != NULL, error = OT_ERROR_INVALID_ARGS);
VerifyOrExit(aFrame != NULL, error = OT_ERROR_INVALID_ARGS);
if (mHead > mTail)
{
otEXPECT_ACTION(newTail < mHead, error = OT_ERROR_NO_BUFS);
VerifyOrExit(newTail < mHead, error = OT_ERROR_NO_BUFS);
}
else if (newTail >= sizeof(mBuffer))
{
newTail -= sizeof(mBuffer);
otEXPECT_ACTION(newTail < mHead, error = OT_ERROR_NO_BUFS);
VerifyOrExit(newTail < mHead, error = OT_ERROR_NO_BUFS);
}
mBuffer[mTail] = aLength;
@@ -79,36 +79,28 @@ exit:
return error;
}
void FrameQueue::Shift(void)
{
if (mHead != mTail)
{
mHead += 1 + mBuffer[mHead];
mHead %= sizeof(mBuffer);
}
}
const uint8_t *FrameQueue::Peek(uint8_t *aFrame, uint8_t &aLength)
const uint8_t *FrameQueue::Shift(uint8_t *aFrame, uint8_t &aLength)
{
const uint8_t *frame = NULL;
uint16_t next;
otEXPECT(mHead != mTail);
VerifyOrExit(mHead != mTail);
aLength = mBuffer[mHead];
next = mHead + 1 + aLength;
if (next >= sizeof(mBuffer))
{
uint16_t half = sizeof(mBuffer) - mHead - 1;
memcpy(aFrame, mBuffer + mHead + 1, half);
memcpy(aFrame + half, mBuffer, aLength - half);
frame = aFrame;
next -= sizeof(mBuffer);
}
else
{
frame = mBuffer + mHead + 1;
}
mHead = next;
exit:
return frame;
+2 -8
View File
@@ -70,12 +70,6 @@ public:
*/
bool IsEmpty(void) const { return mHead == mTail; }
/**
* This method removes one frame from the head.
*
*/
void Shift(void);
/**
* This method pushes one frame into the queue.
*
@@ -89,7 +83,7 @@ public:
otError Push(const uint8_t *aFrame, uint8_t aLength);
/**
* This method gets one frame at head.
* This method shifts one frame at head.
*
* @note aFrame is only used when necessary, always use the returned pointer to access frame data.
*
@@ -99,7 +93,7 @@ public:
* @return A pointer to the frame.
*
*/
const uint8_t *Peek(uint8_t *aFrame, uint8_t &aLength);
const uint8_t *Shift(uint8_t *aFrame, uint8_t &aLength);
private:
enum
+21 -33
View File
@@ -147,21 +147,6 @@ static inline void SuccessOrDie(otError aError)
namespace ot {
/**
* This function returns if the property changed event is *UNSAFE* to be handled during `WaitResponse()`.
*
* If property could trigger another call to `ncpSet()`, it's unsafe.
*
* @param[in] aKey The identifier of the property.
*
* @returns Whether this property is *UNSAFE* to be handled during 'WaitResponse()`.
*
*/
static bool ShouldDefer(spinel_prop_key_t aKey)
{
return aKey == SPINEL_PROP_STREAM_RAW || aKey == SPINEL_PROP_MAC_ENERGY_SCAN_RESULT;
}
static otError SpinelStatusToOtError(spinel_status_t aError)
{
otError ret;
@@ -375,8 +360,9 @@ RadioSpinel::RadioSpinel(void)
, mTxState(kIdle)
, mSockFd(-1)
, mState(OT_RADIO_STATE_DISABLED)
, mAckWait(false)
, mPromiscuous(false)
, mIsAckRequested(false)
, mIsDecoding(false)
, mIsPromiscuous(false)
, mIsReady(false)
{
}
@@ -492,8 +478,7 @@ void RadioSpinel::HandleNotification(const uint8_t *aBuffer, uint16_t aLength)
// Some spinel properties cannot be handled during `WaitResponse()`, we must cache these events.
// `mWaitingTid` is released immediately after received the response. And `mWaitingKey` is be set
// to `SPINEL_PROP_LAST_STATUS` at the end of `WaitResponse()`.
VerifyOrExit(mWaitingKey == SPINEL_PROP_LAST_STATUS || !ShouldDefer(key),
error = mFrameQueue.Push(aBuffer, aLength));
VerifyOrExit(IsSafeToHandleNow(key), error = mFrameQueue.Push(aBuffer, aLength));
HandleValueIs(key, data, static_cast<uint16_t>(len));
break;
@@ -689,7 +674,9 @@ void RadioSpinel::ReadAll(void)
if (rval > 0)
{
mIsDecoding = true;
mHdlcDecoder.Decode(buf, static_cast<uint16_t>(rval));
mIsDecoding = false;
}
}
@@ -699,10 +686,9 @@ void RadioSpinel::ProcessFrameQueue(void)
uint8_t buffer[kMaxSpinelFrame];
const uint8_t *frame;
while ((frame = mFrameQueue.Peek(buffer, length)) != NULL)
while ((frame = mFrameQueue.Shift(buffer, length)) != NULL)
{
HandleNotification(frame, length);
mFrameQueue.Shift();
}
}
@@ -713,7 +699,7 @@ void RadioSpinel::RadioReceive(void)
otShortAddress shortAddress;
otExtAddress extAddress;
VerifyOrExit(mPromiscuous == false, error = OT_ERROR_NONE);
VerifyOrExit(mIsPromiscuous == false, error = OT_ERROR_NONE);
VerifyOrExit((mState == OT_RADIO_STATE_RECEIVE || mState == OT_RADIO_STATE_TRANSMIT), error = OT_ERROR_DROP);
switch (mRxRadioFrame.mPsdu[1] & IEEE802154_DST_ADDR_MASK)
@@ -790,6 +776,7 @@ void RadioSpinel::Process(const fd_set &aReadFdSet, const fd_set &aWriteFdSet)
{
if (FD_ISSET(mSockFd, &aReadFdSet) || !mFrameQueue.IsEmpty())
{
// Handle frames received during WaitResponse()
ProcessFrameQueue();
if (FD_ISSET(mSockFd, &aReadFdSet))
@@ -801,7 +788,7 @@ void RadioSpinel::Process(const fd_set &aReadFdSet, const fd_set &aWriteFdSet)
if (mState == OT_RADIO_STATE_TRANSMIT && mTxState == kDone)
{
mState = OT_RADIO_STATE_RECEIVE;
otPlatRadioTxDone(mInstance, mTransmitFrame, (mAckWait ? &mRxRadioFrame : NULL), mTxError);
otPlatRadioTxDone(mInstance, mTransmitFrame, (mIsAckRequested ? &mRxRadioFrame : NULL), mTxError);
}
}
@@ -820,7 +807,7 @@ otError RadioSpinel::SetPromiscuous(bool aEnable)
uint8_t mode = (aEnable ? SPINEL_MAC_PROMISCUOUS_MODE_NETWORK : SPINEL_MAC_PROMISCUOUS_MODE_OFF);
SuccessOrExit(error = Set(SPINEL_PROP_MAC_PROMISCUOUS_MODE, SPINEL_DATATYPE_UINT8_S, mode));
mPromiscuous = aEnable;
mIsPromiscuous = aEnable;
exit:
return error;
@@ -1070,12 +1057,17 @@ void RadioSpinel::RadioTransmit(void)
otPlatRadioTxStarted(mInstance, mTransmitFrame);
assert(mTxState == kIdle);
mAckWait = isAckRequested(mTransmitFrame->mPsdu);
error = Request(true, SPINEL_CMD_PROP_VALUE_SET, SPINEL_PROP_STREAM_RAW,
mIsAckRequested = isAckRequested(mTransmitFrame->mPsdu);
error = Request(true, SPINEL_CMD_PROP_VALUE_SET, SPINEL_PROP_STREAM_RAW,
SPINEL_DATATYPE_DATA_WLEN_S SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_INT8_S, mTransmitFrame->mPsdu,
mTransmitFrame->mLength, mTransmitFrame->mChannel, mTransmitFrame->mInfo.mRxInfo.mRssi);
if (error)
if (error == OT_ERROR_NONE)
{
mTxState = kSent;
}
else
{
mState = OT_RADIO_STATE_RECEIVE;
@@ -1093,10 +1085,6 @@ void RadioSpinel::RadioTransmit(void)
mTxState = kIdle;
}
else
{
mTxState = kSent;
}
}
otError RadioSpinel::WriteAll(const uint8_t *aBuffer, uint16_t aLength)
@@ -1264,7 +1252,7 @@ void RadioSpinel::HandleTransmitDone(uint32_t aCommand,
aBuffer += unpacked;
aLength -= static_cast<spinel_size_t>(unpacked);
if (mAckWait)
if (mIsAckRequested)
{
VerifyOrExit(aLength > 0, error = OT_ERROR_FAILED);
SuccessOrExit(error = ParseRadioFrame(mRxRadioFrame, aBuffer, aLength));
@@ -1492,7 +1480,7 @@ otRadioCaps otPlatRadioGetCaps(otInstance *aInstance)
bool otPlatRadioGetPromiscuous(otInstance *aInstance)
{
OT_UNUSED_VARIABLE(aInstance);
return sRadioSpinel.GetPromiscuous();
return sRadioSpinel.IsPromiscuous();
}
void platformRadioUpdateFdSet(fd_set *aReadFdSet, fd_set *aWriteFdSet, int *aMaxFd, struct timeval *aTimeout)
+22 -4
View File
@@ -71,7 +71,7 @@ public:
* @retval false Promiscuous mode is disabled.
*
*/
bool GetPromiscuous(void) const { return mPromiscuous; }
bool IsPromiscuous(void) const { return mIsPromiscuous; }
/**
* This method sets the status of promiscuous mode.
@@ -439,6 +439,23 @@ private:
}
void HandleSpinelFrame(const uint8_t *aBuffer, uint16_t aLength);
/**
* This method returns if the property changed event is safe to be handled now.
*
* If a property handler will go up to core stack, it may cause reentrant issue of `Hdlc::Decode()` and
* `WaitResponse()`.
*
* @param[in] aKey The identifier of the property.
*
* @returns Whether this property is safe to be handled now.
*
*/
bool IsSafeToHandleNow(spinel_prop_key_t aKey) const
{
return !((mIsDecoding || mWaitingKey != SPINEL_PROP_LAST_STATUS) &&
(aKey == SPINEL_PROP_STREAM_RAW || aKey == SPINEL_PROP_MAC_ENERGY_SCAN_RESULT));
}
void HandleNotification(const uint8_t *aBuffer, uint16_t aLength);
void HandleValueIs(spinel_prop_key_t aKey, const uint8_t *aBuffer, uint16_t aLength);
@@ -482,9 +499,10 @@ private:
int mSockFd;
otRadioState mState;
bool mAckWait;
bool mPromiscuous;
bool mIsReady;
bool mIsAckRequested : 1; ///< Ack requested.
bool mIsDecoding : 1; ///< Decoding hdlc frames.
bool mIsPromiscuous : 1; ///< Promiscuous mode.
bool mIsReady : 1; ///< NCP ready.
};
} // namespace ot