Re: [PATCH] smbd crash when change notify is disabled

[Stefan Metzmacher]

Am 23.08.2015 um 09:30 schrieb Ralph Böhme:
> On Sat, Aug 15, 2015 at 11:25:47AM +0200, Ralph Böhme wrote:
>> On Fri, Aug 14, 2015 at 11:55:11AM +0200, Ralph Böhme wrote:
>>> Hi Michael,
>>>
>>> On Fri, Aug 14, 2015 at 10:14:13AM +0200, Michael Adam wrote:
>>>> On 2015-08-12 at 12:53 +0200, Ralph Böhme wrote:
>>>>> Hi!
>>>>>
>>>>> Bug <https://bugzilla.samba.org/show_bug.cgi?id=11444> :
>>>>>
>>>>>     If "change notify = no" is set in smb.conf, notify_ctx is
>>>>>     NULL. Then in file_free() we pass notify_ctx (= NULL) to
>>>>>     notify_remove() which doesn't check for that and crashes.
>>>>>
>>>>> Patch attached. Can someone please review and push if ok?
>>>>
>>>> This looks good to me, except for one thing:
>>>>
>>>> Could you use torture_assert_foobar macros in the test suite
>>>> instead of adding yet another incarnation of CHECK_STATUS?
>>>> (torture_assert_ntstatus_equal_goto() and
>>>>  torture_assert_ntstatus_ok_goto().)
>>>
>>> RIP CHECK_STATUS. :) Pushed with this change. Thanks!
>>
>> got the following error from autobuild:
>>
>> UNEXPECTED(failure): samba3.rpc.samba3.netlogon.netlogon(nt4_dc)
>>
>> REASON: Exception: Exception: ../source4/torture/rpc/samba3rpc.c:1426:
>> Expression `auth2(torture, cli, wks_creds)' failed: auth2 failed
>>
>> FAILED (1 failures, 0 errors and 0 unexpected successes in 0
>> testsuites)
>>
>> I tried three times, didn't help. Local autobuild works fine. What's
>> going on?
>>
>> Log:
>> <https://git.samba.org/slow/samba-autobuild/samba.stdout>
> 
> running the patches incrementally in a private autobuild points at the
> last patch being the culprit. No idea why.
> 
> Attached is an updated patchset that simply modifies an existing env
> (simpleserver) instead of adding a new one. This works.
> 
> Please review and push if ok. Thanks!

https://technet.microsoft.com/zh-cn/cc246747 says:
If the underlying object store does not support change notifications,
the server MUST fail this request with STATUS_NOT_SUPPORTED.

So I think we should use NT_STATUS_NOT_SUPPORTED instead of
NT_STATUS_NOT_IMPLEMENTED.

Other than that Reviewed-by: me

Should I change the error code and push?

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150831/bddb3a5e/signature.sig>