-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[snmpd] Fixed snmpd memory leak issue #11812
Conversation
Signed-off-by: Sangita Maity <samaity@linkedin.com>
|
@samaity this change cannot be cherry-picked to 202205 cleanly. Can you create a new PR for 202205 branch? |
This commit could not be cleanly cherry-picked to 202012. Please submit another PR. |
@samaity Is it possible to add a sonic-mgmt testcase to repro the memory leak bug? |
@yxieca I will raise a PR for |
This fixed memory leak in ETHERLIKE-MIB. The fix is not part of net-snmp(5.7.3 version). This PR includes the patch to fix memory leak issue. ``` ke->name in stdup-ed at line 297: n->name = strdup(RTA_DATA(tb[IFLA_IFNAME])); ``` patched the fix. [net-snmp] upstream fix link -> [snmpd}upstream link](net-snmp/net-snmp@ed4e48b) **Before The fix** used valgrind to find memory leak. ``` root@lnos-x1-a-csw06:/# grep "definitely lost" valgrind-out.txt ==493== 4 bytes in 1 blocks are definitely lost in loss record 1 of 333 ==493== 16 bytes in 1 blocks are definitely lost in loss record 25 of 333 ==493== 757 bytes in 71 blocks are definitely lost in loss record 214 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 293 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 294 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 295 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 296 of 333 ==493== definitely lost: 905 bytes in 77 blocks ``` _we can see the memory leak see in stack trace._ -> dot3stats_linux -> get_nlmsg -> strdup https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L277 ``` n = malloc(sizeof(*n)); memset(n, 0, sizeof(*n)); n->ifindex = ifi->ifi_index; n->name = strdup(RTA_DATA(tb[IFLA_IFNAME])); memcpy(&n->stats, RTA_DATA(tb[IFLA_STATS]), sizeof(n->stats)); n->next = kern_db; kern_db = n; return 0; ``` we were not freeing space for EtherLike-MIB.AS interface mib queries were getting increased, we see memory increment. ``` kern_db = ke->next; free(ke); ``` https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L467 ``` ==55== 757 bytes in 71 blocks are definitely lost in loss record 186 of 299 ==55== at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==55== by 0x4EB6E49: strdup (strdup.c:42) ==55== by 0x493F278: get_nlmsg (dot3stats_linux.c:299) ==55== by 0x493F529: rtnl_dump_filter_l.constprop.3 (dot3stats_linux.c:370) ==55== by 0x493FD7A: rtnl_dump_filter (dot3stats_linux.c:401) ==55== by 0x493FD7A: _dot3Stats_netlink_get_errorcntrs (dot3stats_linux.c:424) ==55== by 0x494009F: interface_dot3stats_get_errorcounters (dot3stats_linux.c:530) ==55== by 0x48F6FDA: dot3StatsTable_container_load (dot3StatsTable_data_access.c:330) ==55== by 0x485E76B: _cache_load (cache_handler.c:700) ==55== by 0x485FA37: netsnmp_cache_helper_handler (cache_handler.c:638) ==55== by 0x48720BC: netsnmp_call_handler (agent_handler.c:526) ==55== by 0x48720BC: netsnmp_call_next_handler (agent_handler.c:640) ==55== by 0x4865F75: table_helper_handler (table.c:717) ==55== by 0x4871B66: netsnmp_call_handler (agent_handler.c:526) ==55== by 0x4871B66: netsnmp_call_handlers (agent_handler.c:611) 757 bytes in 71 blocks are definitely lost in loss record 214 of 333 ==493== at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==493== by 0x4EB6E49: strdup (strdup.c:42) ==493== by 0x493F278: ??? (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x493F529: ??? (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x493FD7A: _dot3Stats_netlink_get_errorcntrs (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x494009F: interface_dot3stats_get_errorcounters (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x48F6FDA: dot3StatsTable_container_load (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x485E76B: _cache_load (cache_handler.c:700) ==493== by 0x485FA37: netsnmp_cache_helper_handler (cache_handler.c:638) ==493== by 0x48720BC: netsnmp_call_handler (agent_handler.c:526) ==493== by 0x48720BC: netsnmp_call_next_handler (agent_handler.c:640) ==493== by 0x4865F75: table_helper_handler (table.c:717) ==493== by 0x4871B66: netsnmp_call_handler (agent_handler.c:526) ==493== by 0x4871B66: netsnmp_call_handlers (agent_handler.c:611) ``` ``` **After The fix** no memory leak in valgrind stack trace related to etherlike MIB. ```
@samaity can you please share the PR for 202205? i could not find it and this is critical issue. |
This fixed memory leak in ETHERLIKE-MIB. The fix is not part of net-snmp(5.7.3 version). This PR includes the patch to fix memory leak issue. ``` ke->name in stdup-ed at line 297: n->name = strdup(RTA_DATA(tb[IFLA_IFNAME])); ``` patched the fix. [net-snmp] upstream fix link -> [snmpd}upstream link](net-snmp/net-snmp@ed4e48b) **Before The fix** used valgrind to find memory leak. ``` root@lnos-x1-a-csw06:/# grep "definitely lost" valgrind-out.txt ==493== 4 bytes in 1 blocks are definitely lost in loss record 1 of 333 ==493== 16 bytes in 1 blocks are definitely lost in loss record 25 of 333 ==493== 757 bytes in 71 blocks are definitely lost in loss record 214 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 293 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 294 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 295 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 296 of 333 ==493== definitely lost: 905 bytes in 77 blocks ``` _we can see the memory leak see in stack trace._ -> dot3stats_linux -> get_nlmsg -> strdup https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L277 ``` n = malloc(sizeof(*n)); memset(n, 0, sizeof(*n)); n->ifindex = ifi->ifi_index; n->name = strdup(RTA_DATA(tb[IFLA_IFNAME])); memcpy(&n->stats, RTA_DATA(tb[IFLA_STATS]), sizeof(n->stats)); n->next = kern_db; kern_db = n; return 0; ``` we were not freeing space for EtherLike-MIB.AS interface mib queries were getting increased, we see memory increment. ``` kern_db = ke->next; free(ke); ``` https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L467 ``` ==55== 757 bytes in 71 blocks are definitely lost in loss record 186 of 299 ==55== at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==55== by 0x4EB6E49: strdup (strdup.c:42) ==55== by 0x493F278: get_nlmsg (dot3stats_linux.c:299) ==55== by 0x493F529: rtnl_dump_filter_l.constprop.3 (dot3stats_linux.c:370) ==55== by 0x493FD7A: rtnl_dump_filter (dot3stats_linux.c:401) ==55== by 0x493FD7A: _dot3Stats_netlink_get_errorcntrs (dot3stats_linux.c:424) ==55== by 0x494009F: interface_dot3stats_get_errorcounters (dot3stats_linux.c:530) ==55== by 0x48F6FDA: dot3StatsTable_container_load (dot3StatsTable_data_access.c:330) ==55== by 0x485E76B: _cache_load (cache_handler.c:700) ==55== by 0x485FA37: netsnmp_cache_helper_handler (cache_handler.c:638) ==55== by 0x48720BC: netsnmp_call_handler (agent_handler.c:526) ==55== by 0x48720BC: netsnmp_call_next_handler (agent_handler.c:640) ==55== by 0x4865F75: table_helper_handler (table.c:717) ==55== by 0x4871B66: netsnmp_call_handler (agent_handler.c:526) ==55== by 0x4871B66: netsnmp_call_handlers (agent_handler.c:611) 757 bytes in 71 blocks are definitely lost in loss record 214 of 333 ==493== at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==493== by 0x4EB6E49: strdup (strdup.c:42) ==493== by 0x493F278: ??? (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x493F529: ??? (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x493FD7A: _dot3Stats_netlink_get_errorcntrs (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x494009F: interface_dot3stats_get_errorcounters (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x48F6FDA: dot3StatsTable_container_load (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x485E76B: _cache_load (cache_handler.c:700) ==493== by 0x485FA37: netsnmp_cache_helper_handler (cache_handler.c:638) ==493== by 0x48720BC: netsnmp_call_handler (agent_handler.c:526) ==493== by 0x48720BC: netsnmp_call_next_handler (agent_handler.c:640) ==493== by 0x4865F75: table_helper_handler (table.c:717) ==493== by 0x4871B66: netsnmp_call_handler (agent_handler.c:526) ==493== by 0x4871B66: netsnmp_call_handlers (agent_handler.c:611) ``` ``` **After The fix** no memory leak in valgrind stack trace related to etherlike MIB. ```
This fixed memory leak in ETHERLIKE-MIB. The fix is not part of net-snmp(5.7.3 version). This PR includes the patch to fix memory leak issue. ``` ke->name in stdup-ed at line 297: n->name = strdup(RTA_DATA(tb[IFLA_IFNAME])); ``` patched the fix. [net-snmp] upstream fix link -> [snmpd}upstream link](net-snmp/net-snmp@ed4e48b) **Before The fix** used valgrind to find memory leak. ``` root@lnos-x1-a-csw06:/# grep "definitely lost" valgrind-out.txt ==493== 4 bytes in 1 blocks are definitely lost in loss record 1 of 333 ==493== 16 bytes in 1 blocks are definitely lost in loss record 25 of 333 ==493== 757 bytes in 71 blocks are definitely lost in loss record 214 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 293 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 294 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 295 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 296 of 333 ==493== definitely lost: 905 bytes in 77 blocks ``` _we can see the memory leak see in stack trace._ -> dot3stats_linux -> get_nlmsg -> strdup https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L277 ``` n = malloc(sizeof(*n)); memset(n, 0, sizeof(*n)); n->ifindex = ifi->ifi_index; n->name = strdup(RTA_DATA(tb[IFLA_IFNAME])); memcpy(&n->stats, RTA_DATA(tb[IFLA_STATS]), sizeof(n->stats)); n->next = kern_db; kern_db = n; return 0; ``` we were not freeing space for EtherLike-MIB.AS interface mib queries were getting increased, we see memory increment. ``` kern_db = ke->next; free(ke); ``` https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L467 ``` ==55== 757 bytes in 71 blocks are definitely lost in loss record 186 of 299 ==55== at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==55== by 0x4EB6E49: strdup (strdup.c:42) ==55== by 0x493F278: get_nlmsg (dot3stats_linux.c:299) ==55== by 0x493F529: rtnl_dump_filter_l.constprop.3 (dot3stats_linux.c:370) ==55== by 0x493FD7A: rtnl_dump_filter (dot3stats_linux.c:401) ==55== by 0x493FD7A: _dot3Stats_netlink_get_errorcntrs (dot3stats_linux.c:424) ==55== by 0x494009F: interface_dot3stats_get_errorcounters (dot3stats_linux.c:530) ==55== by 0x48F6FDA: dot3StatsTable_container_load (dot3StatsTable_data_access.c:330) ==55== by 0x485E76B: _cache_load (cache_handler.c:700) ==55== by 0x485FA37: netsnmp_cache_helper_handler (cache_handler.c:638) ==55== by 0x48720BC: netsnmp_call_handler (agent_handler.c:526) ==55== by 0x48720BC: netsnmp_call_next_handler (agent_handler.c:640) ==55== by 0x4865F75: table_helper_handler (table.c:717) ==55== by 0x4871B66: netsnmp_call_handler (agent_handler.c:526) ==55== by 0x4871B66: netsnmp_call_handlers (agent_handler.c:611) 757 bytes in 71 blocks are definitely lost in loss record 214 of 333 ==493== at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==493== by 0x4EB6E49: strdup (strdup.c:42) ==493== by 0x493F278: ??? (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x493F529: ??? (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x493FD7A: _dot3Stats_netlink_get_errorcntrs (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x494009F: interface_dot3stats_get_errorcounters (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x48F6FDA: dot3StatsTable_container_load (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x485E76B: _cache_load (cache_handler.c:700) ==493== by 0x485FA37: netsnmp_cache_helper_handler (cache_handler.c:638) ==493== by 0x48720BC: netsnmp_call_handler (agent_handler.c:526) ==493== by 0x48720BC: netsnmp_call_next_handler (agent_handler.c:640) ==493== by 0x4865F75: table_helper_handler (table.c:717) ==493== by 0x4871B66: netsnmp_call_handler (agent_handler.c:526) ==493== by 0x4871B66: netsnmp_call_handlers (agent_handler.c:611) ``` ``` **After The fix** no memory leak in valgrind stack trace related to etherlike MIB. ```
@samaity can you confirm if 202205 PR has been raised? |
@samaity This commit could not be cleanly cherry-picked to 202012. Please submit another PR. |
#### Why I did it This fixed memory leak in ETHERLIKE-MIB. The fix is not part of net-snmp(5.7.3 version). This PR includes the patch to fix memory leak issue. ``` ke->name in stdup-ed at line 297: n->name = strdup(RTA_DATA(tb[IFLA_IFNAME])); ``` #### How I did it patched the fix. [net-snmp] upstream fix link -> [snmpd|upstream link](net-snmp/net-snmp@ed4e48b) #### How to verify it **Before The fix** used valgrind to find memory leak. ``` root@lnos-x1-a-csw06:/# grep "definitely lost" valgrind-out.txt ==493== 4 bytes in 1 blocks are definitely lost in loss record 1 of 333 ==493== 16 bytes in 1 blocks are definitely lost in loss record 25 of 333 ==493== 757 bytes in 71 blocks are definitely lost in loss record 214 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 293 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 294 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 295 of 333 ==493== 1,168 (32 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 296 of 333 ==493== definitely lost: 905 bytes in 77 blocks ``` _we can see the memory leak see in stack trace._ -> dot3stats_linux -> get_nlmsg -> strdup https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L277 ``` n = malloc(sizeof(*n)); memset(n, 0, sizeof(*n)); n->ifindex = ifi->ifi_index; n->name = strdup(RTA_DATA(tb[IFLA_IFNAME])); memcpy(&n->stats, RTA_DATA(tb[IFLA_STATS]), sizeof(n->stats)); n->next = kern_db; kern_db = n; return 0; ``` we were not freeing space for EtherLike-MIB.AS interface mib queries were getting increased, we see memory increment. ``` kern_db = ke->next; free(ke); ``` https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L467 ``` ==55== 757 bytes in 71 blocks are definitely lost in loss record 186 of 299 ==55== at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==55== by 0x4EB6E49: strdup (strdup.c:42) ==55== by 0x493F278: get_nlmsg (dot3stats_linux.c:299) ==55== by 0x493F529: rtnl_dump_filter_l.constprop.3 (dot3stats_linux.c:370) ==55== by 0x493FD7A: rtnl_dump_filter (dot3stats_linux.c:401) ==55== by 0x493FD7A: _dot3Stats_netlink_get_errorcntrs (dot3stats_linux.c:424) ==55== by 0x494009F: interface_dot3stats_get_errorcounters (dot3stats_linux.c:530) ==55== by 0x48F6FDA: dot3StatsTable_container_load (dot3StatsTable_data_access.c:330) ==55== by 0x485E76B: _cache_load (cache_handler.c:700) ==55== by 0x485FA37: netsnmp_cache_helper_handler (cache_handler.c:638) ==55== by 0x48720BC: netsnmp_call_handler (agent_handler.c:526) ==55== by 0x48720BC: netsnmp_call_next_handler (agent_handler.c:640) ==55== by 0x4865F75: table_helper_handler (table.c:717) ==55== by 0x4871B66: netsnmp_call_handler (agent_handler.c:526) ==55== by 0x4871B66: netsnmp_call_handlers (agent_handler.c:611) 757 bytes in 71 blocks are definitely lost in loss record 214 of 333 ==493== at 0x483577F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==493== by 0x4EB6E49: strdup (strdup.c:42) ==493== by 0x493F278: ??? (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x493F529: ??? (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x493FD7A: _dot3Stats_netlink_get_errorcntrs (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x494009F: interface_dot3stats_get_errorcounters (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x48F6FDA: dot3StatsTable_container_load (in /usr/lib/x86_64-linux-gnu/libnetsnmpmibs.so.30.0.3) ==493== by 0x485E76B: _cache_load (cache_handler.c:700) ==493== by 0x485FA37: netsnmp_cache_helper_handler (cache_handler.c:638) ==493== by 0x48720BC: netsnmp_call_handler (agent_handler.c:526) ==493== by 0x48720BC: netsnmp_call_next_handler (agent_handler.c:640) ==493== by 0x4865F75: table_helper_handler (table.c:717) ==493== by 0x4871B66: netsnmp_call_handler (agent_handler.c:526) ==493== by 0x4871B66: netsnmp_call_handlers (agent_handler.c:611) ``` ``` **After The fix** no memory leak in valgrind stack trace related to etherlike MIB. ```
@qiluo-msft @yxieca @liat-grozovik @lguohan already raised for 202012 -> [https://github.com/sonic-net/sonic-buildimage/pull/13387](#13387) 202205 -> #13388 (merged too) |
Signed-off-by: Sangita Maity samaity@linkedin.com
Why I did it
This fixed memory leak in ETHERLIKE-MIB. The fix is not part of net-snmp(5.7.3 version). This PR includes the patch to fix memory leak issue.
How I did it
patched the fix.
[net-snmp] upstream fix link -> snmpd}upstream link
How to verify it
Before The fix
used valgrind to find memory leak.
we can see the memory leak see in stack trace.
-> dot3stats_linux -> get_nlmsg -> strdup
https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c
https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L277
we were not freeing space for EtherLike-MIB.AS interface mib queries were getting increased, we see memory increment.
https://github.com/net-snmp/net-snmp/blob/v5.7.3/agent/mibgroup/etherlike-mib/data_access/dot3stats_linux.c#L467
Which release branch to backport (provide reason below if selected)