From 1a2d5f0458fba6b505793930da29cd091e52a293 Mon Sep 17 00:00:00 2001 From: gabekassel Date: Thu, 22 Aug 2024 16:18:37 -0700 Subject: [PATCH] [core] valgrind reported memory access bugs (#9833) * 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 * 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 * 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 --- src/core/thread/key_manager.cpp | 1 + src/core/thread/key_manager.hpp | 2 +- src/posix/platform/backtrace.cpp | 2 ++ src/posix/platform/netif.cpp | 26 +++++++++++++++----------- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/core/thread/key_manager.cpp b/src/core/thread/key_manager.cpp index 9f40c9ec2..dd12eaa2d 100644 --- a/src/core/thread/key_manager.cpp +++ b/src/core/thread/key_manager.cpp @@ -65,6 +65,7 @@ const uint8_t KeyManager::kTrelInfoString[] = {'T', 'h', 'r', 'e', 'a', 'd', 'O' void SecurityPolicy::SetToDefault(void) { + Clear(); mRotationTime = kDefaultKeyRotationTime; SetToDefaultFlags(); } diff --git a/src/core/thread/key_manager.hpp b/src/core/thread/key_manager.hpp index 56a64acb3..6091c4a05 100644 --- a/src/core/thread/key_manager.hpp +++ b/src/core/thread/key_manager.hpp @@ -68,7 +68,7 @@ namespace ot { * Represents Security Policy Rotation and Flags. * */ -class SecurityPolicy : public otSecurityPolicy, public Equatable +class SecurityPolicy : public otSecurityPolicy, public Equatable, public Clearable { public: /** diff --git a/src/posix/platform/backtrace.cpp b/src/posix/platform/backtrace.cpp index 1f697ca57..988718d7a 100644 --- a/src/posix/platform/backtrace.cpp +++ b/src/posix/platform/backtrace.cpp @@ -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; diff --git a/src/posix/platform/netif.cpp b/src/posix/platform/netif.cpp index 12bee4256..37eb4cb05 100644 --- a/src/posix/platform/netif.cpp +++ b/src/posix/platform/netif.cpp @@ -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) + 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)) { - errorMsg = reinterpret_cast(RTA_DATA(rta)); - break; - } - else - { - LogDebg("Ignoring netlink response attribute %d (request#%u)", rta->rta_type, requestSeq); + if (rta->rta_type == NLMSGERR_ATTR_MSG) + { + errorMsg = reinterpret_cast(RTA_DATA(rta)); + break; + } + else + { + LogDebg("Ignoring netlink response attribute %d (request#%u)", rta->rta_type, requestSeq); + } } }