[nat64] fix message leak when NAT64 translator is enabled (#8592)

This commit fixes a potential message leak when NAT64 translator
feature is enabled in `Ip6::ProcessReceiveCallback()` where a cloned
copy of message is not freed if `TranslateFromIp6()` returns an
error or if `mReceiveIp4DatagramCallback` is not set.

This commit updates how we track and free the message in this method
(to help simplify the code and avoid similar situations). Variable
`message` points to the `Message` instance we own in this method. If
we can take ownership of `aMessage`, we use it as `message`.
Otherwise, we may create a clone of it. `message` will be set to
`nullptr` if the message ownership is transferred to an invoked
callback. At the end of this method we free `message` if it is not
`nullptr` (indicating it was not passed to a callback.
This commit is contained in:
Abtin Keshavarzian
2022-12-29 17:16:33 -08:00
committed by GitHub
parent 3aab651adc
commit 25c6e0ec89
+20 -7
View File
@@ -998,13 +998,26 @@ Error Ip6::ProcessReceiveCallback(Message &aMessage,
Message::Ownership aMessageOwnership)
{
Error error = kErrorNone;
Message *message = &aMessage;
Message *message = nullptr;
#if OPENTHREAD_CONFIG_IP6_BR_COUNTERS_ENABLE
Header header;
IgnoreError(header.ParseFrom(aMessage));
#endif
// `message` points to the `Message` instance we own in this
// method. If we can take ownership of `aMessage`, we use it as
// `message`. Otherwise, we may create a clone of it and use as
// `message`. `message` variable will be set to `nullptr` if the
// message ownership is transferred to an invoked callback. At
// the end of this method we free `message` if it is not `nullptr`
// indicating it was not passed to a callback.
if (aMessageOwnership == Message::kTakeCustody)
{
message = &aMessage;
}
VerifyOrExit(aOrigin != kFromHostDisallowLoopBack, error = kErrorNoRoute);
VerifyOrExit(mReceiveIp6DatagramCallback.IsSet(), error = kErrorNoRoute);
@@ -1029,6 +1042,7 @@ Error Ip6::ProcessReceiveCallback(Message &aMessage,
if (mIcmp.ShouldHandleEchoRequest(aMessageInfo))
{
Icmp::Header icmp;
IgnoreError(aMessage.Read(aMessage.GetOffset(), icmp));
// do not pass ICMP Echo Request messages
@@ -1081,24 +1095,23 @@ Error Ip6::ProcessReceiveCallback(Message &aMessage,
ExitNow(error = kErrorDrop);
case Nat64::Translator::kForward:
VerifyOrExit(mReceiveIp4DatagramCallback.IsSet(), error = kErrorNoRoute);
// Pass message to callback transferring its ownership.
mReceiveIp4DatagramCallback.Invoke(message);
message = nullptr;
ExitNow();
}
#endif
// Pass message to callback transferring its ownership.
mReceiveIp6DatagramCallback.Invoke(message);
message = nullptr;
#if OPENTHREAD_CONFIG_IP6_BR_COUNTERS_ENABLE
UpdateBorderRoutingCounters(header, aMessage.GetLength(), /* aIsInbound */ false);
#endif
exit:
if ((error != kErrorNone) && (aMessageOwnership == Message::kTakeCustody))
{
aMessage.Free();
}
FreeMessage(message);
return error;
}