Skip to content

Commit

Permalink
Fix the stack-overflow issue in AclOrch::getTableById() (#1073)
Browse files Browse the repository at this point in the history
* Fix the stack-overflow issue in AclOrch::getTableById()

In case the table_id and m_mirrorTableId/m_mirrorTableId are empty
The function would be called recursively without ending.

```
If we have some rule that does not have table defined, it will trigger this issue.

(gdb) bt
#0  0x00007f77ac5af3a9 in swss::Logger::write (this=0x7f77ac801ec0 <swss::Logger::getInstance()::m_logger>, prio=swss::Logger::SWSS_DEBUG, fmt=0x7f77ac5f1690 ":> %s: enter")
    at logger.cpp:209
#1  0x00000000004a5792 in AclOrch::getTableById (this=this@entry=0x1c722a0, table_id="") at aclorch.cpp:2617
#2  0x00000000004a59d1 in AclOrch::getTableById (this=this@entry=0x1c722a0, table_id="") at aclorch.cpp:2634
#3  0x00000000004a59d1 in AclOrch::getTableById (this=this@entry=0x1c722a0, table_id="") at aclorch.cpp:2634
...
#20944 0x00000000004a59d1 in AclOrch::getTableById (this=this@entry=0x1c722a0, table_id="") at aclorch.cpp:2634
#20945 0x00000000004ad3ce in AclOrch::doAclRuleTask (this=this@entry=0x1c722a0, consumer=...) at aclorch.cpp:2437
#20946 0x00000000004b04cd in AclOrch::doTask (this=0x1c722a0, consumer=...) at aclorch.cpp:2141
#20947 0x00000000004231b2 in Orch::doTask (this=0x1c722a0) at orch.cpp:369
#20948 0x000000000041c4e9 in OrchDaemon::start (this=this@entry=0x1c19960) at orchdaemon.cpp:376
#20949 0x0000000000409ffc in main (argc=<optimized out>, argv=0x7ffe2e392d68) at main.cpp:295

(gdb) p table_id
$1 = ""
(gdb) p m_mirrorTableId
$2 = ""
(gdb) p m_mirrorV6TableId
$3 = ""
```

Signed-off-by: Zhenggen Xu <[email protected]>
  • Loading branch information
zhenggen-xu authored Jan 29, 2020
1 parent be867dc commit bb7a372
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2836,6 +2836,13 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
string op = kfvOp(t);

SWSS_LOG_INFO("OP: %s, TABLE_ID: %s, RULE_ID: %s", op.c_str(), table_id.c_str(), rule_id.c_str());

if (table_id.empty())
{
SWSS_LOG_WARN("ACL rule with RULE_ID: %s is not valid as TABLE_ID is empty", rule_id.c_str());
it = consumer.m_toSync.erase(it);
continue;
}

if (op == SET_COMMAND)
{
Expand Down Expand Up @@ -3016,6 +3023,12 @@ sai_object_id_t AclOrch::getTableById(string table_id)
{
SWSS_LOG_ENTER();

if (table_id.empty())
{
SWSS_LOG_WARN("table_id is empty");
return SAI_NULL_OBJECT_ID;
}

for (auto it : m_AclTables)
{
if (it.second.id == table_id)
Expand Down

0 comments on commit bb7a372

Please sign in to comment.