[PATCH] tfork fix issues with waiter process termination

[Andrew Bartlett]

On Sat, 2017-09-16 at 07:52 -0700, Ralph Böhme via samba-technical
wrote:
> On Sat, Sep 16, 2017 at 10:13:37AM +0000, Gary Lockyer wrote:
> > > Your fix for the status fd is spot on, I'd like to change the event fd fix
> > > however. Depending on whether tfork_event_fd() has been called, ownership of the
> > > event fd should be transferred to the caller.
> > 
> > That makes more sense as it will reduce the chances of file descriptor
> > leaks.
> > > 
> > > Please take a look at attached patchset. The FIXUP commits are some minor
> > > fixes. The event fd changes are in the "lib/util: only close the event_fd in
> > > tfork if the caller didn't call tfork_event_fd()" commit.
> > 
> > I'm happy with the revised patch set.
> 
> great!
> 
> We also need a backport for 4.7, don't we? metze already created a bug for this
> one, so I'm going to add the bug URL and push later on if I don't hear any
> objections.
> 
> https://bugzilla.samba.org/show_bug.cgi?id=13037

A BUG URL is always a good thing, but I'm not sure we need the backport
critically.

The reason this was never noticed is that in the current use, a sibling
(actually cousin) worker process still holding the FD from the parent
is quite rare.  You would get it if you ran 'samba -M single' for smbd
and winbindd, but thankfully everything is marked close-on-exec, so no
duplicate is found then, and the monitor process closes all FDs using a
loop.

The new use case, to handle a forked-but-not-execed samba child in a
process model re-factor is what caught this case.

I'll let Gary tell more of the tale of how we found this, but the
whiteboard sessions were quite epic, tracing who had what read and
write FDs open!

Thanks!

Andrew Bartlett
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba