Skip to content

Commit

Permalink
[buffermgrd] [portsyncd] [vxlanmgrd] [mirrororch] fix a set of memory…
Browse files Browse the repository at this point in the history
… issues (sonic-net#2221)

- What I did
Fixed the following memory issues:

[buffermgrd] the "runningProfileName" is used after the object it references is deleted
[portsyncd] the 'if_ni' is not freed
[vxlanmgrd] the 'info' is used after the object it references is deleted
[mirrororch] removed unneeded calloc()

- Why I did it
To fix memory usage issues

- How I verified it
Run the tests that was used to find the issues and checked the ASAN report

Signed-off-by: Yakiv Huryk <[email protected]>
  • Loading branch information
Yakiv-Huryk authored May 11, 2022
1 parent b3e97a0 commit eff725f
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 16 deletions.
2 changes: 1 addition & 1 deletion cfgmgr/buffermgrdyn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2853,7 +2853,7 @@ task_process_status BufferMgrDynamic::handleSingleBufferPgEntry(const string &ke
// For del command:
// 1. Removing it from APPL_DB
// 2. Update internal caches
string &runningProfileName = bufferPg.running_profile_name;
string runningProfileName = bufferPg.running_profile_name;
string &configProfileName = bufferPg.configured_profile_name;

if (!m_supportRemoving)
Expand Down
2 changes: 1 addition & 1 deletion cfgmgr/vxlanmgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,8 @@ bool VxlanMgr::doVxlanDeleteTask(const KeyOpFieldsValuesTuple & t)
SWSS_LOG_WARN("Vxlan %s hasn't been created ", info.m_vxlan.c_str());
}

m_vnetCache.erase(it);
SWSS_LOG_INFO("Delete vxlan %s", info.m_vxlan.c_str());
m_vnetCache.erase(it);
return true;
}

Expand Down
3 changes: 1 addition & 2 deletions orchagent/mirrororch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -797,8 +797,7 @@ bool MirrorOrch::setUnsetPortMirror(Port port,
if (set)
{
port_attr.value.objlist.count = 1;
port_attr.value.objlist.list = reinterpret_cast<sai_object_id_t *>(calloc(port_attr.value.objlist.count, sizeof(sai_object_id_t)));
port_attr.value.objlist.list[0] = sessionId;
port_attr.value.objlist.list = &sessionId;
}
else
{
Expand Down
17 changes: 5 additions & 12 deletions portsyncd/linksync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#include <errno.h>
#include <system_error>
#include <sys/socket.h>
#include <linux/if.h>
#include <net/if.h>
#include <netlink/route/link.h>
#include "logger.h"
#include "netmsg.h"
Expand Down Expand Up @@ -34,23 +34,16 @@ const string LAG_PREFIX = "PortChannel";
extern set<string> g_portSet;
extern bool g_init;

struct if_nameindex
{
unsigned int if_index;
char *if_name;
};
extern "C" { extern struct if_nameindex *if_nameindex (void) __THROW; }

LinkSync::LinkSync(DBConnector *appl_db, DBConnector *state_db) :
m_portTableProducer(appl_db, APP_PORT_TABLE_NAME),
m_portTable(appl_db, APP_PORT_TABLE_NAME),
m_statePortTable(state_db, STATE_PORT_TABLE_NAME),
m_stateMgmtPortTable(state_db, STATE_MGMT_PORT_TABLE_NAME)
{
struct if_nameindex *if_ni, *idx_p;
if_ni = if_nameindex();
std::shared_ptr<struct if_nameindex> if_ni(if_nameindex(), if_freenameindex);
struct if_nameindex *idx_p;

for (idx_p = if_ni;
for (idx_p = if_ni.get();
idx_p != NULL && idx_p->if_index != 0 && idx_p->if_name != NULL;
idx_p++)
{
Expand Down Expand Up @@ -121,7 +114,7 @@ LinkSync::LinkSync(DBConnector *appl_db, DBConnector *state_db) :
}
}

for (idx_p = if_ni;
for (idx_p = if_ni.get();
idx_p != NULL && idx_p->if_index != 0 && idx_p->if_name != NULL;
idx_p++)
{
Expand Down

0 comments on commit eff725f

Please sign in to comment.