https://gitlab.com/samba-team/samba/-/blob/master/nsswitch/wbinfo.c?ref_type=heads#L1571 In function "wbinfo_lookup_sids" in successful completion var "sids" memory leak If there is an error, "TALLOC_FREE(sids);" is called when the function exits early
TALLOC_FREE() is rarely called in this module for variables that are initiated via "talloc_tos()". The calling function "main" calls "talloc_free(frame)". But it is more optimal if all the functions inside will call "TALLOC_FREE()" for variables initiated through "talloc_tos()"
I can do this myself and send a merge request if it is correct.
talloc_tos() is indeed both a protection against long-term memleaks as well as just a convenience function. For long-running daemons we have the expectation that the toplevel talloc_stackframe is very frequently cleaned up, so a precise and early TALLOC_FREE() of objects is not strictly necessary, although it is of course cleaner to do so. In short-lived tools like wbinfo we're even more laid back, the process exit will properly clean up everything behind us. Bug 157711 is potentially worse: pam_winbind is a library linked into other programs, so we can't properly expect the talloc_stackframe to be cleaned up. It is highly likely that it is done at the end of the pam routines, in pam and nsswitch we need to be much more careful to not introduce leaks. So -- worthwhile findings, but not something we would stop the presses for. And -- MRs against gitlab.com/samba-team are the right thing to do.
Thanks for the detailed description