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

[Richard Sharpe]

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.

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));

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)