Fix the corrupted Trie routing tree
The routing table may potentially become corrupted during the routing
table construction as the vector element pointer becomes invalid if the
vector is resized [1].
http/routing/trie.hpp#L241:
```
ContainedType& node = nodes[idx];
size_t* param = &node.stringParamChild;
if (str1 == "<path>")
{
param = &node.pathParamChild;
}
if (*param == 0U)
{
L249:
*param = newNode(); // <---
}
idx = *param;
```
Here, `newNodes()` at L249 may resize the vector of `nodes[]` and thus
the reference of `nodes[idx]` becomes invalid and thus the previously
saved the pointer of `param` is invalid.
The similar issue is also at sub_route_trie construction [5].
This problem may be shown during CI/valgrind test depending on the order
of route setups in [2].
For example, for the commit 39574 [3], if `requestsRoutesAssembly()` is
added earlier than `requestRoutesProcessorCollection()`, it causes
CI/valgrind test fails [3].
The error looks like [4].
[1] https://github.com/openbmc/bmcweb/blob/master/http/routing/trie.hpp#L241
[2] https://github.com/openbmc/bmcweb/blob/master/redfish-core/src/redfish.cpp
[3] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/39574
[4] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/39574/comment/15e652e0_f8881ffc/
[5] https://github.com/openbmc/bmcweb/blob/master/redfish-core/include/sub_route_trie.hpp#L160
Tested:
- CI with https://gerrit.openbmc.org/c/openbmc/bmcweb/+/39574 passes
after rebase of having earlier `requestsRoutesAssembly()`.
- Redfish Service Validator passes
Change-Id: I349777dfab65f2d41eb5db25796d82322b3c36cc
Signed-off-by: Myung Bae <myungbae@us.ibm.com>
diff --git a/http/routing/trie.hpp b/http/routing/trie.hpp
index bfa441a..8bb5593 100644
--- a/http/routing/trie.hpp
+++ b/http/routing/trie.hpp
@@ -238,17 +238,24 @@
continue;
}
found = true;
- ContainedType& node = nodes[idx];
- size_t* param = &node.stringParamChild;
if (str1 == "<path>")
{
- param = &node.pathParamChild;
+ if (nodes[idx].pathParamChild == 0U)
+ {
+ unsigned newNodeIdx = newNode();
+ nodes[idx].pathParamChild = newNodeIdx;
+ }
+ idx = nodes[idx].pathParamChild;
}
- if (*param == 0U)
+ else
{
- *param = newNode();
+ if (nodes[idx].stringParamChild == 0U)
+ {
+ unsigned newNodeIdx = newNode();
+ nodes[idx].stringParamChild = newNodeIdx;
+ }
+ idx = nodes[idx].stringParamChild;
}
- idx = *param;
url.remove_prefix(str1.size());
break;
diff --git a/redfish-core/include/sub_route_trie.hpp b/redfish-core/include/sub_route_trie.hpp
index f930600..c8f9fa7 100644
--- a/redfish-core/include/sub_route_trie.hpp
+++ b/redfish-core/include/sub_route_trie.hpp
@@ -157,17 +157,24 @@
continue;
}
found = true;
- ContainedType& node = this->nodes[idx];
- size_t* param = &node.stringParamChild;
if (str1 == "<path>")
{
- param = &node.pathParamChild;
+ if (this->nodes[idx].pathParamChild == 0U)
+ {
+ unsigned newNodeIdx = this->newNode();
+ this->nodes[idx].pathParamChild = newNodeIdx;
+ }
+ idx = this->nodes[idx].pathParamChild;
}
- if (*param == 0U)
+ else
{
- *param = this->newNode();
+ if (this->nodes[idx].stringParamChild == 0U)
+ {
+ unsigned newNodeIdx = this->newNode();
+ this->nodes[idx].stringParamChild = newNodeIdx;
+ }
+ idx = this->nodes[idx].stringParamChild;
}
- idx = *param;
url.remove_prefix(str1.size());
break;