Skip to content
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 #13387

Open
wants to merge 1 commit into
base: 202012
Choose a base branch
from

Conversation

samaity
Copy link
Collaborator

@samaity samaity commented Jan 17, 2023

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.

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

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.

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.
```
@lguohan
Copy link
Collaborator

lguohan commented Jan 19, 2023

@samaity , we need backport to other branches as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants