espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-10-19 14:08:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

[spinel] fix wrong arguments for various data types in Logger (#12557)

Browse Source 
This commit fixes improper argument usage for SPINEL_DATATYPE_DATA_WLEN_S,
SPINEL_DATATYPE_DATA_S, SPINEL_DATATYPE_EUI64_S, and SPINEL_DATATYPE_UTF8_S
in Logger::LogSpinelFrame.

Key changes:
- Use pointer-to-pointer for data and unsigned int pointer for length
  when using spinel_datatype_unpack with data types.
- Switch to spinel_datatype_unpack_in_place() when unpacking into local
  buffers or structures (e.g., for EUI-64 addresses and MAC keys).
- Remove redundant length arguments for SPINEL_DATATYPE_UTF8_S.
- Refactor SPINEL_PROP_RCP_MAC_KEY to use otMacKey and in-place unpacking.

These fixes prevent potential crashes, stack corruption, and incorrect
data parsing in the Spinel logging utility.
This commit is contained in:
Yakun Xu
2026-02-25 21:44:29 -06:00
committed by GitHub
parent 3758ac4b48
commit ec1eb19656
1 changed files with 27 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/lib/spinel/logger.cpp
+27 -23
View File
@@ -446,6 +446,7 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
uint16_t flags; uint16_t flags;
int8_t noiseFloor; int8_t noiseFloor;
unsigned int receiveError; unsigned int receiveError;
unsigned int psduLength;
unpacked = spinel_datatype_unpack(data, len, unpacked = spinel_datatype_unpack(data, len,
SPINEL_DATATYPE_DATA_WLEN_S // Frame SPINEL_DATATYPE_DATA_WLEN_S // Frame
@@ -459,10 +460,12 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
) SPINEL_DATATYPE_STRUCT_S( // Vendor-data ) SPINEL_DATATYPE_STRUCT_S( // Vendor-data
SPINEL_DATATYPE_UINT_PACKED_S // Receive error SPINEL_DATATYPE_UINT_PACKED_S // Receive error
), ),
&frame.mPsdu, &frame.mLength, &frame.mInfo.mRxInfo.mRssi, &noiseFloor, nullptr, &psduLength, &frame.mInfo.mRxInfo.mRssi, &noiseFloor, &flags,
&flags, &frame.mChannel, &frame.mInfo.mRxInfo.mLqi, &frame.mChannel, &frame.mInfo.mRxInfo.mLqi,
&frame.mInfo.mRxInfo.mTimestamp, &receiveError); &frame.mInfo.mRxInfo.mTimestamp, &receiveError);
VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE);
frame.mLength = static_cast<uint16_t>(psduLength);
start += Snprintf(start, static_cast<uint32_t>(end - start), ", len:%u, rssi:%d ...", frame.mLength, start += Snprintf(start, static_cast<uint32_t>(end - start), ", len:%u, rssi:%d ...", frame.mLength,
frame.mInfo.mRxInfo.mRssi); frame.mInfo.mRxInfo.mRssi);
OT_UNUSED_VARIABLE(start); // Avoid static analysis error OT_UNUSED_VARIABLE(start); // Avoid static analysis error
@@ -476,10 +479,11 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
} }
else if (cmd == SPINEL_CMD_PROP_VALUE_SET) else if (cmd == SPINEL_CMD_PROP_VALUE_SET)
{ {
bool csmaCaEnabled; unsigned int psduLength;
bool isHeaderUpdated; bool csmaCaEnabled;
bool isARetx; bool isHeaderUpdated;
bool skipAes; bool isARetx;
bool skipAes;
unpacked = spinel_datatype_unpack( unpacked = spinel_datatype_unpack(
data, len, data, len,
@@ -493,11 +497,13 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
SPINEL_DATATYPE_BOOL_S // SkipAes SPINEL_DATATYPE_BOOL_S // SkipAes
SPINEL_DATATYPE_UINT32_S // TxDelay SPINEL_DATATYPE_UINT32_S // TxDelay
SPINEL_DATATYPE_UINT32_S, // TxDelayBaseTime SPINEL_DATATYPE_UINT32_S, // TxDelayBaseTime
&frame.mPsdu, &frame.mLength, &frame.mChannel, &frame.mInfo.mTxInfo.mMaxCsmaBackoffs, nullptr, &psduLength, &frame.mChannel, &frame.mInfo.mTxInfo.mMaxCsmaBackoffs,
&frame.mInfo.mTxInfo.mMaxFrameRetries, &csmaCaEnabled, &isHeaderUpdated, &isARetx, &skipAes, &frame.mInfo.mTxInfo.mMaxFrameRetries, &csmaCaEnabled, &isHeaderUpdated, &isARetx, &skipAes,
&frame.mInfo.mTxInfo.mTxDelay, &frame.mInfo.mTxInfo.mTxDelayBaseTime); &frame.mInfo.mTxInfo.mTxDelay, &frame.mInfo.mTxInfo.mTxDelayBaseTime);
VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE);
frame.mLength = static_cast<uint16_t>(psduLength);
start += Snprintf(start, static_cast<uint32_t>(end - start), start += Snprintf(start, static_cast<uint32_t>(end - start),
", len:%u, channel:%u, maxbackoffs:%u, maxretries:%u ...", frame.mLength, frame.mChannel, ", len:%u, channel:%u, maxbackoffs:%u, maxretries:%u ...", frame.mLength, frame.mChannel,
frame.mInfo.mTxInfo.mMaxCsmaBackoffs, frame.mInfo.mTxInfo.mMaxFrameRetries); frame.mInfo.mTxInfo.mMaxCsmaBackoffs, frame.mInfo.mTxInfo.mMaxFrameRetries);
@@ -547,9 +553,8 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
case SPINEL_PROP_NEST_STREAM_MFG: case SPINEL_PROP_NEST_STREAM_MFG:
{ {
const char *output; const char *output;
size_t outputLen;
unpacked = spinel_datatype_unpack(data, len, SPINEL_DATATYPE_UTF8_S, &output, &outputLen); unpacked = spinel_datatype_unpack(data, len, SPINEL_DATATYPE_UTF8_S, &output);
VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE);
start += Snprintf(start, static_cast<uint32_t>(end - start), ", diag:%s", output); start += Snprintf(start, static_cast<uint32_t>(end - start), ", diag:%s", output);
} }
@@ -566,11 +571,11 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
otMacKey nextKey; otMacKey nextKey;
unsigned int nextKeyLen = sizeof(otMacKey); unsigned int nextKeyLen = sizeof(otMacKey);
unpacked = spinel_datatype_unpack(data, len, unpacked = spinel_datatype_unpack_in_place(
SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_DATA_WLEN_S data, len,
SPINEL_DATATYPE_DATA_WLEN_S SPINEL_DATATYPE_DATA_WLEN_S, SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_DATA_WLEN_S SPINEL_DATATYPE_DATA_WLEN_S
&keyIdMode, &keyId, prevKey.m8, &prevKeyLen, currKey.m8, &currKeyLen, SPINEL_DATATYPE_DATA_WLEN_S,
nextKey.m8, &nextKeyLen); &keyIdMode, &keyId, prevKey.m8, &prevKeyLen, currKey.m8, &currKeyLen, nextKey.m8, &nextKeyLen);
VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE);
start += Snprintf(start, static_cast<uint32_t>(end - start), start += Snprintf(start, static_cast<uint32_t>(end - start),
", keyIdMode:%u, keyId:%u, prevKey:***, currKey:***, nextKey:***", keyIdMode, keyId); ", keyIdMode:%u, keyId:%u, prevKey:***, currKey:***, nextKey:***", keyIdMode, keyId);
@@ -681,11 +686,10 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
case SPINEL_PROP_MAC_SCAN_MASK: case SPINEL_PROP_MAC_SCAN_MASK:
{ {
constexpr uint8_t kNumChannels = 16; const uint8_t *channels;
uint8_t channels[kNumChannels]; spinel_size_t size;
spinel_size_t size;
unpacked = spinel_datatype_unpack(data, len, SPINEL_DATATYPE_DATA_S, channels, &size); unpacked = spinel_datatype_unpack(data, len, SPINEL_DATATYPE_DATA_S, &channels, &size);
VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE);
start += Snprintf(start, static_cast<uint32_t>(end - start), ", channels:"); start += Snprintf(start, static_cast<uint32_t>(end - start), ", channels:");
@@ -702,7 +706,7 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
uint8_t m8[OT_EXT_ADDRESS_SIZE]; uint8_t m8[OT_EXT_ADDRESS_SIZE];
uint8_t flags; uint8_t flags;
unpacked = spinel_datatype_unpack( unpacked = spinel_datatype_unpack_in_place(
data, len, SPINEL_DATATYPE_UINT16_S SPINEL_DATATYPE_EUI64_S SPINEL_DATATYPE_UINT8_S, &saddr, m8, &flags); data, len, SPINEL_DATATYPE_UINT16_S SPINEL_DATATYPE_EUI64_S SPINEL_DATATYPE_UINT8_S, &saddr, m8, &flags);
VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE);
@@ -716,10 +720,10 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx)
{ {
if (cmd == SPINEL_CMD_PROP_VALUE_INSERT) if (cmd == SPINEL_CMD_PROP_VALUE_INSERT)
{ {
uint8_t channel; uint8_t channel;
int16_t actualPower; int16_t actualPower;
uint8_t *rawPowerSetting; const uint8_t *rawPowerSetting;
unsigned int rawPowerSettingLength; unsigned int rawPowerSettingLength;
unpacked = spinel_datatype_unpack( unpacked = spinel_datatype_unpack(
data, len, SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_INT16_S SPINEL_DATATYPE_DATA_WLEN_S, &channel, data, len, SPINEL_DATATYPE_UINT8_S SPINEL_DATATYPE_INT16_S SPINEL_DATATYPE_DATA_WLEN_S, &channel,