Bug 229020

Summary: Potential memory leak in sbin/umount
Product: Base System Reporter: Thomas Barabosch <thomas.barabosch>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: New ---    
Severity: Affects Many People CC: trueos
Priority: --- Keywords: patch
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Possible patch none

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 freebsd_committer freebsd_triage 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.