Skip to content

Commit

Permalink
[core] valgrind reported memory access bugs (openthread#9833)
Browse files Browse the repository at this point in the history
* posix: check for nlmsg error tlv attributes

if we couldn't set NETLINK_EXT_ACK, there's no extra nlmsg attributes in
the error. avoid UB and walking uninitialized memory by checking the
flag for those attributes. for us, this avoids segfaults and in one
instance, an infinite loop while walking the non-existant attributes.

Signed-off-by: Nick Owens <[email protected]>

* posix: zero initialize sigaction struct before use

this removes a valgrind warning about use of uninitialized memory in a
syscall when backtrace is enabled.

Signed-off-by: Nick Owens <[email protected]>

* key_manager: zero initialize otSecurityPolicy

valgrind reports that otSecurityPolicy is used uninitialized, so just
make it zero.

clear all bytes when setting to default

Signed-off-by: Nick Owens <[email protected]>
  • Loading branch information
gabekassel authored Aug 22, 2024
1 parent 24e9306 commit 1a2d5f0
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 12 deletions.
1 change: 1 addition & 0 deletions src/core/thread/key_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const uint8_t KeyManager::kTrelInfoString[] = {'T', 'h', 'r', 'e', 'a', 'd', 'O'

void SecurityPolicy::SetToDefault(void)
{
Clear();
mRotationTime = kDefaultKeyRotationTime;
SetToDefaultFlags();
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/thread/key_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ namespace ot {
* Represents Security Policy Rotation and Flags.
*
*/
class SecurityPolicy : public otSecurityPolicy, public Equatable<SecurityPolicy>
class SecurityPolicy : public otSecurityPolicy, public Equatable<SecurityPolicy>, public Clearable<SecurityPolicy>
{
public:
/**
Expand Down
2 changes: 2 additions & 0 deletions src/posix/platform/backtrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ void platformBacktraceInit(void)
{
struct sigaction sigact;

memset(&sigact, 0, sizeof(struct sigaction));

sigact.sa_sigaction = &signalCritical;
sigact.sa_flags = SA_RESTART | SA_SIGINFO | SA_NOCLDWAIT;

Expand Down
26 changes: 15 additions & 11 deletions src/posix/platform/netif.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1751,19 +1751,23 @@ static void HandleNetlinkResponse(struct nlmsghdr *msg)
requestPayloadLength = NLMSG_PAYLOAD(&err->msg, 0);
}

rtaLength = NLMSG_PAYLOAD(msg, sizeof(struct nlmsgerr)) - requestPayloadLength;

for (struct rtattr *rta = ERR_RTA(err, requestPayloadLength); RTA_OK(rta, rtaLength);
rta = RTA_NEXT(rta, rtaLength))
// Only extract inner TLV error if flag is set
if (msg->nlmsg_flags & NLM_F_ACK_TLVS)
{
if (rta->rta_type == NLMSGERR_ATTR_MSG)
{
errorMsg = reinterpret_cast<const char *>(RTA_DATA(rta));
break;
}
else
rtaLength = NLMSG_PAYLOAD(msg, sizeof(struct nlmsgerr)) - requestPayloadLength;

for (struct rtattr *rta = ERR_RTA(err, requestPayloadLength); RTA_OK(rta, rtaLength);
rta = RTA_NEXT(rta, rtaLength))
{
LogDebg("Ignoring netlink response attribute %d (request#%u)", rta->rta_type, requestSeq);
if (rta->rta_type == NLMSGERR_ATTR_MSG)
{
errorMsg = reinterpret_cast<const char *>(RTA_DATA(rta));
break;
}
else
{
LogDebg("Ignoring netlink response attribute %d (request#%u)", rta->rta_type, requestSeq);
}
}
}

Expand Down

0 comments on commit 1a2d5f0

Please sign in to comment.