[PATCH] smbd crash when change notify is disabled

Michael Adam obnox at samba.org
Mon Aug 31 11:45:33 UTC 2015


On 2015-08-31 at 13:26 +0200, Ralph Böhme wrote:
> On Mon, Aug 31, 2015 at 12:44:24PM +0200, Stefan Metzmacher wrote:
> > 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.
> 
> +1

ACK

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


More information about the samba-technical mailing list