Skip to content

Commit

Permalink
[dbus] fix Scan return type and match introspect (#1814)
Browse files Browse the repository at this point in the history
The dbus encoding and decoding methods were attempting to send the
incorrect amount of data over dbus. This breaks the dbus introspect
contract and causes runtime errors.

To fix the issue:
- Re-introduce the entire `ActiveScanResult` data structure to the
  introspect document.
- Coerce the RSSI value into an int16 when encoding and decoding
  an `ActiveScanResult`.
  • Loading branch information
jdswensen authored and jwhui committed May 17, 2023
1 parent 54383a7 commit d78e768
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 7 deletions.
4 changes: 2 additions & 2 deletions src/dbus/common/dbus_message_helper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ template <> struct DBusTypeTrait<ChildInfo>
template <> struct DBusTypeTrait<ActiveScanResult>
{
// struct of { uint64, string, uint64, array<uint8>, uint16, uint16, uint8,
// uint8, uint8, uint8, bool, bool }
static constexpr const char *TYPE_AS_STRING = "(tstayqqyyyybb)";
// int16, uint8, uint8, bool, bool }
static constexpr const char *TYPE_AS_STRING = "(tstayqqynyybb)";
};

template <> struct DBusTypeTrait<EnergyScanResult>
Expand Down
15 changes: 13 additions & 2 deletions src/dbus/common/dbus_message_helper_openthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ otbrError DBusMessageExtract(DBusMessageIter *aIter, ActiveScanResult &aScanResu
DBusMessageIter sub;
otbrError error = OTBR_ERROR_NONE;

// Local variable to help convert an RSSI value.
// Dbus doesn't have the concept of a signed byte
int16_t rssi = 0;

VerifyOrExit(dbus_message_iter_get_arg_type(aIter) == DBUS_TYPE_STRUCT, error = OTBR_ERROR_DBUS);
dbus_message_iter_recurse(aIter, &sub);

Expand All @@ -64,12 +68,16 @@ otbrError DBusMessageExtract(DBusMessageIter *aIter, ActiveScanResult &aScanResu
SuccessOrExit(error = DBusMessageExtract(&sub, aScanResult.mPanId));
SuccessOrExit(error = DBusMessageExtract(&sub, aScanResult.mJoinerUdpPort));
SuccessOrExit(error = DBusMessageExtract(&sub, aScanResult.mChannel));
SuccessOrExit(error = DBusMessageExtract(&sub, aScanResult.mRssi));
SuccessOrExit(error = DBusMessageExtract<int16_t>(&sub, rssi));
SuccessOrExit(error = DBusMessageExtract(&sub, aScanResult.mLqi));
SuccessOrExit(error = DBusMessageExtract(&sub, aScanResult.mVersion));
SuccessOrExit(error = DBusMessageExtract(&sub, aScanResult.mIsNative));
SuccessOrExit(error = DBusMessageExtract(&sub, aScanResult.mDiscover));

// Double check the value is within int8 bounds and cast back
VerifyOrExit((rssi <= INT8_MAX) && (rssi >= INT8_MIN), error = OTBR_ERROR_PARSE);
aScanResult.mRssi = static_cast<int8_t>(rssi);

dbus_message_iter_next(aIter);
error = OTBR_ERROR_NONE;
exit:
Expand All @@ -89,7 +97,10 @@ otbrError DBusMessageEncode(DBusMessageIter *aIter, const ActiveScanResult &aSca
SuccessOrExit(error = DBusMessageEncode(&sub, aScanResult.mPanId));
SuccessOrExit(error = DBusMessageEncode(&sub, aScanResult.mJoinerUdpPort));
SuccessOrExit(error = DBusMessageEncode(&sub, aScanResult.mChannel));
SuccessOrExit(error = DBusMessageEncode(&sub, aScanResult.mRssi));

// Dbus doesn't have a signed byte, cast into an int16
SuccessOrExit(error = DBusMessageEncode(&sub, static_cast<int16_t>(aScanResult.mRssi)));

SuccessOrExit(error = DBusMessageEncode(&sub, aScanResult.mLqi));
SuccessOrExit(error = DBusMessageEncode(&sub, aScanResult.mVersion));
SuccessOrExit(error = DBusMessageEncode(&sub, aScanResult.mIsNative));
Expand Down
13 changes: 10 additions & 3 deletions src/dbus/server/introspect.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,22 @@
<literallayout>
struct {
uint64 ext_address
string network_name
uint64 ext_panid
uint8[] steering_data
uint16 panid
uint16 channel
uint16 rssi
uint16 joiner_udp_port
uint8 channel
int16 rssi
uint8 lqi
uint8 version
bool is_native
bool is_joinable
}
</literallayout>
-->
<method name="Scan">
<arg name="scan_result" type="a(tqqqy)" direction="out"/>
<arg name="scan_result" type="a(tstayqqynyybb)" direction="out"/>
</method>

<!-- Energy Scan: Perform a Thread energy scan.
Expand Down

0 comments on commit d78e768

Please sign in to comment.