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

Stefan Metzmacher metze at samba.org
Mon Aug 31 10:44:24 UTC 2015


Am 31.08.2015 um 11:25 schrieb Michael Adam:
> On 2015-08-31 at 11:20 +0200, Stefan Metzmacher wrote:
>> 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.

I just noticed that this change is just for notify_remove()
and the only caller ignored the result.

I also found that the MS-CIFS says:

If the server does not support the NT_TRANSACT_NOTIFY_CHANGE subcommand,
it can return an
error response with STATUS_NOT_IMPLEMENTED (ERRDOS/ERRbadfunc) in
response to an
NT_TRANSACT_NOTIFY_CHANGE Request. Alternatively, it can send
STATUS_NOTIFY_ENUM_DIR
(ERRDOS/ERR_NOTIFY_ENUM_DIR) to cause the client to enumerate the directory

I'll push the patchset unchanged for now.

Changing the status for SMB2 to NOT_SUPPORTED
or changing both to ENUM_DIR can be a later change.

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/5734a66a/signature.sig>


More information about the samba-technical mailing list