Created attachment 9610 [details] gdb backtrace and object details In two cases we have seen a segfault in replmd_check_urgent_objectclass due to the objectClass attribute missing on a deleted (\0ADEL) object. In the second case the segfault occurred during a "samba-tool dbcheck --fix", which is a bit of a letdown ;-). Patch suggestions will be attached below.
Created attachment 9611 [details] 0001-Avoid-segfault-when-objectClass-is-missing.patch
Created attachment 9612 [details] 0001-Improve-objectClass-validation.patch This adds additional checks to two other calls of replmd_check_urgent_objectclass in the same source file.
Created attachment 9729 [details] give an error if the objectClass is missing The attached patch makes this a fatal error, which I think is more appropriate so we don't propagate this error further. We need to find where this error comes from, and stop the rot. What do you think?
Created attachment 9737 [details] give an error if the objectClass is missing (oops, I uploaded the wrong patch).
Wouldn't this also stop "samba-tool dbcheck --fix" from doing it's job?
Also ldbdel probably fails? What about ldbmodify?
Just a note for others who see this to say that we have added in checks to prevent this in Samba master, by preventing inbound replication of these corrupt objects. Some of these patches should be backported.
(In reply to comment #7) > Just a note for others who see this to say that we have added in checks to > prevent this in Samba master, by preventing inbound replication of these > corrupt objects. > > Some of these patches should be backported. And what about dbchecker.py patches? imo those are very important too
Created attachment 10088 [details] Temporary patches for v4-1-test Once I got the master for through autobuild, I can upload the patches with cherry-pick -x information, but the content will be the same so this can be reviewed now.
(In reply to comment #9) > Created attachment 10088 [details] > Temporary patches for v4-1-test > > Once I got the master for through autobuild, > I can upload the patches with cherry-pick -x information, > but the content will be the same so this can be reviewed now. What format is that patch in? It isn't clear-text...
Comment on attachment 10088 [details] Temporary patches for v4-1-test It's bz2
What is different in this patch set that isn't in the patches I've reviewed on the other bugs?
(In reply to comment #12) > What is different in this patch set that isn't in the patches I've reviewed on > the other bugs? It's too large as plain text. The top 30 patches from https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=cf33119fb9ca7f390fc2eb This is the large one "selftest: Add tests for dbcheck detection and removal of partial objects" 34 files changed, 80719 insertions(+), 3 deletions(-) https://git.samba.org/?p=metze/samba/wip.git;a=commit;h=d06491e77ec65255481fee70c4
The first 3 patches duplicate bug #8449. Otherwise, it's fine. I do wonder what is left in master after all this, and wish we had instead the ability to quickly (eg 1month) do a 4.2 with all these changes, but given the realities doing a DRS roll-up release is the best option. There has been a lot of good fixes in master in the past couple of months in particular.
(In reply to comment #14) > The first 3 patches duplicate bug #8449. I'll have a look. > Otherwise, it's fine. I do wonder what is left in master after all this, and > wish we had instead the ability to quickly (eg 1month) do a 4.2 with all these > changes, but given the realities doing a DRS roll-up release is the best > option. There has been a lot of good fixes in master in the past couple of > months in particular. The autolockout patches are still a difference.
(In reply to comment #15) > (In reply to comment #14) > > The first 3 patches duplicate bug #8449. > > I'll have a look. I don't see any of the patches of #8849 ...
(In reply to comment #16) > (In reply to comment #15) > > (In reply to comment #14) > > > The first 3 patches duplicate bug #8449. > > > > I'll have a look. > > I don't see any of the patches of #8849 ... The patches in the attachement are: cf33119fb9ca7f390fc2eb70d2ee872c32d53397 CHPX s4:dsdb/kcc: use SHOW_RECYCLED instead of SHOW_DELETED in when deleting tombstone/deleted objects 92dac5636282efa09e69f8967a4ceb51eda53463 CHPX s4:dsdb/extended_dn_in: don't force DSDB_SEARCH_SHOW_RECYCLED ad48882454dc1545948d82808e08e27b2bc7aa8d CHPX s4:dsdb/schema_load: make error message more verbose 6fa410e5e977696d7db64cec8a2bd0ba491e7480 dbcheck: Ensure dbcheck can operate with --attrs set b0a6e1fa7fbbf4b93e148b7af79779437e051ccc kerberos: Remove un-used event context argument from smb_krb5_init_context() 37a9e1c0e33b21cb916dee73000b611cf0ea7bf3 dsdb: Specify no event context to smb_krb5_init_context() in dsdb a7ce65c9d3fdcd6b59a397441cb9c3e5e84417e4 dsdb: Add DSDB_SEARCH_ONE_ONLY support to dsdb_module_search*() 17ed9a2d7087119250b881159eef8a6401b3867c dsdb: Do not permit nested event loops when in a transaction, use a nested event context 5322cd20a26c5280fd0c9ca5e6fb0e1e4144c1bd dsdb: Rename private_data to rootdse_private_data in rootdse c97cae27c4dbd60d75b45d4f5613ec8e54158394 dsdb: Add more tests for DN+String and DN+Binary comparisons d06491e77ec65255481fee70c4f9e3c674d6a4cb selftest: Add tests for dbcheck detection and removal of partial objects 994622c27aae3f17c04032ab3a85688ab5bf27af dsdb: Make it harder to corrupt the database by requiring DBCHECK or RELAX for final object deletion db3d3131793322e3768cbc4fa3ff5c9840fba0d8 build: Exclude source4/selftest/provisions/release-4-1-0rc3 from the tarball 9ae61dd50aa2fdbe79ee70f126dd80da990e2df8 dbcheck: Directly call dn.get_rdn_{val,name}() for clarity and consistency 9995c9a6323c16d727407c946eacbe252f6df2ea dbchecker: verify and fix broken dn values 50783ac5ae44d44b5157a59ca653eb1f15aba7f7 dbchecker: make the deleted objects container detection more generic 0a962501c860302b386ebaff5437d121af15ccfd dsdb: Do not refresh the schema using the wrong event context af2c0063177cd98851f45aa88ceefbff599c84bc dsdb: Do not store a struct ldb_dn in struct schema_data d428a9875c396878302292d2ecc84cebc484ecc2 samba-tool dbcheck: handle missing objectClass 868389f86a34e33bc298bcc0b6f2869e0cccbfe9 dsdb: Improve missing objectClass handling 5e42c79bc2865772adec7b228722c4e158a61fac dsdb: Improve errors and checks for missing objectClass values 3f131379bb98611dcf66670489666ee165c27c84 dsdb: Clarify how the DSDB_REPL_FLAG_PRIORITISE_INCOMING flag works cf12cc006eba99bd60deef82fff3c425048fac99 dsdb: Do not update notify_uSN until the transaction is genuinely committed to the DB 16b91e7610997b9756731e15cbf81852d8b405f9 dsdb: Further assert that we always have an objectClass and an rDN 12e3496b44e83defcfa3abf9360105bcaf3eb5fd dsdb: Ensure to sort replPropertyMetaData as UNSIGNED, not SIGNED quantities 19341443bb0197ff3657bfa6e749d6f207551a30 s4:samdb: respect SEARCH_FLAG_PRESERVEONDELETE fc311f4a65dda344ddb6cd1bbff3f7ad93494b82 s4-samldb: Do not allow deletion of objects with RID < 1000 ae61d25a6972ccc36a5811d0fc188e6f1404a62a dsdb: Use dsdb_next_callback() rather than a no-op per-module callback 088d844aed5b47f019e673c33a29f5cadc10ff0c s4-dsdb: instanceType NC_HEAD is only allowed combined with WRITE for an originating add operation c128b8a35a9682f272308748073fb186b9487e1c s4:dsdb/repl: make use of dcerpc_binding_handle_is_connected()
Created attachment 10094 [details] Patches for v4-1-test The same as the temporary patches but with cherry-pick information
Comment on attachment 10094 [details] Patches for v4-1-test I guess I was mis-reading my local tree where I did the backport for those patches, Sorry about that.
(In reply to comment #18) > Created attachment 10094 [details] > Patches for v4-1-test > > The same as the temporary patches but with cherry-pick information Pushed to autobuild-v4-1-test.
(In reply to comment #20) > (In reply to comment #18) > > Created attachment 10094 [details] [details] > > Patches for v4-1-test > > > > The same as the temporary patches but with cherry-pick information > > Pushed to autobuild-v4-1-test. Pushed to v4-1-test. Closing out bug report. Thanks!