[PATCH] smbd crash when change notify is disabled

[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.
> 
> Other than that Reviewed-by: me
> 
> Should I change the error code and push?

Barring this change: reviewed-by me, too.

Ralph: Thanks for converting to the torture_assert macros!
And sorry for the delay (I was on vacation...).

Michael
-------------- 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/457bbdc1/attachment.sig>