Fix some resource leak issues #104
Conversation
There was a problem hiding this comment.
Overall, it looks like this catches several possible memory leaks in the error paths, but it highlights a couple other places that similar changes are needed. Also, care needs to be exercised that this doesn't introduce any worse behaviors like a double-free that can trigger a crash instead.
There was a problem hiding this comment.
This PR appears to fix several memory leaks that occur in the error return paths of multiple functions and should be merged once all issues are resolved, but there does appear to be one critical blocker that must be resolved first. There is one memory free that is on the successful code path for a data structure that appears to have a lifetime beyond this function call. Please review the comment and apply an appropriate fix as needed.
lldp_dcbx_nl.c
Outdated
| @@ -540,6 +545,7 @@ int get_dcb_capabilities(char *ifname, | |||
| if (rta_parent != rta_child) | |||
| LLDPAD_DBG("rta pointers are off\n"); | |||
|
|
|||
| err_out: | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
This PR fixes several memory leaks and I don't see anymore blocking issues that prevent this from being merged at this time. It is also passing the full CI build suite with no failures:
https://github.com/penguin359/openlldp/actions/runs/10368328451
The arglens and argvallens variables are realloced in the loop. So when there is a failure, both of them should be freed. Let's make numargs = 0 and goto out directly. Fixes: 60bc0ad ("lldpad: handle more than 8 lldptool parameters") Signed-off-by: Hangbin Liu <[email protected]>
The mod and dud variables are allocated but not freed during out_err, which could cause a resource leak. Ensure that these memory allocations are properly freed to prevent this issue. Fixes: a37b7e0 ("lldpad: initial git commit") Signed-off-by: Hangbin Liu <[email protected]>
The ptlv variable is not freed when an error occurs. Let's ensure it is freed to avoid a resource leak. Also move the !ptlv guard to the top, right after the copy_ptlv(du). Fixes: 36a641e ("ecp22 add notify module call back function") Signed-off-by: Hangbin Liu <[email protected]>
Overwriting tlv with NULL directly may lead to a memory leak, as the storage that tlv points to is not freed. Use free_unpkd_tlv instead to properly free the memory. Fixes: fe410e2 ("lldpad: implementation of IEEE 802.1Qbg in lldpad, part 2") Signed-off-by: Hangbin Liu <[email protected]>
The nlh variable is allocated but not freed in some error out situations, which could cause a resource leak. Ensure nlh is properly freed to prevent this issue. Fixes: a37b7e0 ("lldpad: initial git commit") Signed-off-by: Hangbin Liu <[email protected]>
|
Applied, thanks. I will dedicate some time next week to backport these to 1.1 branch. |
Fix issues where some resources are allocated but not freed, preventing potential resource leaks.