From bb7a37228af415cd4a46c0f21d7003ca5a71f45a Mon Sep 17 00:00:00 2001 From: zhenggen-xu Date: Tue, 28 Jan 2020 21:46:44 -0800 Subject: [PATCH] Fix the stack-overflow issue in AclOrch::getTableById() (#1073) * 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 , 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=, 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 --- orchagent/aclorch.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 09a5778c91..52855c87fd 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -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) { @@ -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)