|Summary:||Potential memory leak in sbin/umount|
|Product:||Base System||Reporter:||Thomas Barabosch <thomas.barabosch>|
|Component:||bin||Assignee:||freebsd-bugs (Nobody) <bugs>|
|Severity:||Affects Many People||CC:||trueos|
Description Thomas Barabosch 2018-06-14 19:56:17 UTC
Created attachment 194258 [details] Possible patch There may be a potential memory leak in sbin/umount. In function umountfs there is a call to getaddrinfo. According to getaddrinfo.3: "All of the information returned by getaddrinfo() is dynamically allo- cated: the addrinfo structures themselves as well as the socket address structures and the canonical host name strings included in the addrinfo structures. Memory allocated for the dynamically allocated structures created by a successful call to getaddrinfo() is released by the freeaddrinfo() func- tion. The ai pointer should be a addrinfo structure created by a call to getaddrinfo()." However, the whole file umount.c does not make a single call to freeaddrinfo(). It would be better to free the addrinfo with freeaddrinfo to prevent a potential memory leak. Can you confirm this problem? I've attached a patch as a possible solution.
Comment 1 Conrad Meyer 2018-06-15 01:43:25 UTC
addrinfo allocation is limited to at most one per NFS mountpoint to be unmounted, and memory is freed at program exit. I don't see a problem.
Comment 2 Thomas Barabosch 2018-06-16 13:05:35 UTC
(In reply to Conrad Meyer from comment #1) Conrad, good point! Nevertheless, the base system code should be a good example how to properly use the library functions, in this case getaddrinfo/freeaddrinfo.
Comment 3 Trenton Schulz 2020-09-19 17:27:00 UTC
Looked at this as part of the bug squash. Conrad isn't wrong that the leak here is mostly harmless for umount, but Thomas has a point that it would be nice to have the code correct. If you could create a review for this on phabricator (reviews.freebsd.org) and tag Tom Jones (thj@), we'll try to shepherd this through.