A patch to add support for smbd_server_connection_terminate_ex to allow for explicit debug messages in some cases

[Andrew Bartlett]

On Mon, 2013-05-27 at 19:38 -0700, Richard Sharpe wrote:
> On Mon, May 27, 2013 at 7:27 PM, Andrew Bartlett <abartlet at samba.org> wrote:
> > On Mon, 2013-05-27 at 19:22 -0700, Richard Sharpe wrote:
> >> On Mon, May 27, 2013 at 7:14 PM, Andrew Bartlett <abartlet at samba.org> wrote:
> >> > On Mon, 2013-05-27 at 19:11 -0700, Richard Sharpe wrote:
> >> >> On Sun, May 26, 2013 at 4:48 PM, Andrew Bartlett <abartlet at samba.org> wrote:
> >> >> > On Fri, 2013-05-17 at 01:24 -0700, Richard Sharpe wrote:
> >> >> >> Hi folks,
> >> >> >>
> >> >> >> Here is a patch that makes life easier. I have modified one exit,
> >> >> >> although there are a couple more where we should also scream on exit.
> >> >> >>
> >> >> >> If it looks OK, please push.
> >> >> >
> >> >> > I still don't understand why you don't want to just call exit_server()
> >> >> > in this case.
> >> >>
> >> >> Good point. Out of respect for the way things are currently done I
> >> >> tried to follow some semblance of the same.
> >> >
> >> > So the main difference with exit_server() is that it is for really fatal
> >> > conditions that should just never happen, where we want a backtrace and
> >> > core for inspection, to ensure it doesn't happen again.  Is this one of
> >> > those?
> >>
> >> I believe we have seen cases in the field where smbds are exiting
> >> without telling us anything. One was where writev was returning EINVAL
> >> due to a bug in the ixgbe driver (and it was an ancient version of the
> >> driver), but there is likely to be other cases.
> >>
> >> Samba was hiding this info from us in some misguided attempt to not
> >> spam log files. (IMO, of course.)
> >>
> >> Of course, there are cases where it should exit silently, but not, I
> >> believe when there have been protocol violations or in cases like I
> >> have described above where an underlying bug caused a problem.
> >
> > We shouldn't be too loud for protocol violations, as that can cause a
> > trivial DoS as a client spams us with invalid stuff, but if the layers
> > below us give us errors that 'can't happen', we should yell from the
> > rooftops.
> >
> >> It took a week for us (where I work) to figure out what the problem
> >> was because Samba was being reticent about telling us the problem and
> >> it was not until I had a customer system at a level 10 debug that I
> >> had a hint as to the problem.
> >
> > If this can't happen in normal operation, then exit_server() sounds
> > correct to me.  We can always change it back, or refine the cases
> > later.
> >
> > Specifically for writev giving EINVAL, aside from driver bugs (always a
> > good reason to panic), the reasons given here imply programmer error,
> > which implies exit_server() as the appropriate action:
> >
> >       EINVAL fd is attached to an object which is unsuitable for
> > writing; or the file was opened with the O_DIRECT flag, and either the
> > address specified in buf, the value specified in
> >               count, or the current file offset is not suitably aligned.
> >
> > I hope this helps,
> 
> Struggling up from my alcohol-induced haze (An Australian Merlot) I
> think I understand.
> 
> However, exit_server does not allow printf style messages, although I
> could wrap it with a macro.

Just update it to take a printf - the existing users will just not have
any printf arguments.  There is no need for a macro, just declare it
correctly and handle the printf inside, like many similar functions do. 

> What would be useful is something like:
> 
> if the error is one of these expected errors, exit_server_cleanly
> otherwise exit_server_printf("some syscall returned %s\n", strerror(errno));

Just:
 exit_server("some syscall returned %s\n", strerror(errno));

would be better.

Andrew Bartlett
-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org