hi William,

Let's discuss it on this bug, which is restricted for now to core Samba developers and you.

This is for two reasons:

1. There might be security implications, where for example a malicious user can crash the RPC server.

2. It might be useful for you to share installation information that you don't really want to be public.

At the moment I don't think this will be a security issue -- more likely the result of some oddity the domain has acquired over the years.

William: are all the stack traces the same?


It looks like the auth_session_info attached to the ldb is unexpectedly not a valid talloc object. I don't know why.

Within the same samba version and logging configuration they appear to be identical.
On the 'main' DC which is currently running ubuntu 20.04.1 the backtrace is identical down to and including hex values whilst it was on the default 4.11.6 samba packages, the hex values changed (but nothing else) when I upgraded to the 4.12.6 apt.van-belle.nl packages, then the hex values changed again when I changed the logging settings in smb.conf.
Checking two other DCs which have self-compiled samba 4.10.10 their backtraces appear to be identical except for what appears to be an offset in the hex values.

(In reply to William Ross from comment #3)

If you can start a DC with `valgrind --trace-children=yes samba`, we might learn more. It will also be very slow, which could either be good, because the bug may be triggered by the increased load, or bad, because it will be slow and never get around to crashing.

Alternatively/also (on a different DC), you could build with `./configure --enable-debug`, and add a line like this to the [global] section of smb.conf:

        panic action = $SAMBA_SRC_PATH/selftest/gdb_backtrace %d

where $SAMBA_SRC_PATH is the actual path to samba source directory. The tracebacks it produces will be more voluminous and detailed.

Thanks Douglas. Have been away for a few days, will rebuild the DC tomorrow with enable-debug and report back.

Created attachment 16223 [details]
Log file containing PANIC with enable-debug and gdb_backtrace

Installed the enable-debug build on CentOS 8 VM and rejoined to the domain (in place of previous DC with the same name) last night (10/09/2020). This morning there was a PANIC in the log file. Had added #panic action = /root/samba-4.12.6/selftest/gdb_backtrace %d to smb.conf as per instructions.
Please see the output attached.

(In reply to William Ross from comment #6)
Thanks William.

The traceback has a tiny amount of perhaps not highly sensitive information (basically, host names), so I will mark it as private, meaning the attachment will remain hidden from non-samba developers after the bug is opened up.

Andrew: is it that the session_info is somehow being freed while the ldb is still in use, even though this is guarded against around lib/ldb-samba/ldb_wrap.c:274 (v4-12-stable)? Or is this ldb not one of those ldbs?

I will look a bit further.

I'm going to try starting soda (DC at main site) with `valgrind --trace-children=yes samba` to produce additional data.

These are the PANICs on the domain controller soda (the main one) between installation of samba 4.12.6 on 10/09/2020 and today (30/09/2020):

2020/09/11 05:54:01
2020/09/11 11:31:21
2020/09/17 09:06:19
2020/09/17 11:03:58
2020/09/17 13:36:46
2020/09/17 17:26:53
2020/09/18 08:02:26
2020/09/18 11:29:44
2020/09/19 01:03:23
2020/09/19 14:45:51
2020/09/24 01:04:32

Its curious to me how they cluster around thursdays/fridays. It will be interesting to see if this repeats tomorrow/friday.

Of those PANICs they all look like the ones previously shared except for one which references 'FFFFFFF8DsCrackNames':

[2020/09/11 11:31:21.064923,  0] ../../lib/util/util_runcmd.c:352(samba_runcmd_io_handler)
  /usr/local/samba/sbin/samba_kcc: WARNING: The "lanman auth" option is deprecated
  DsCrackNames: Unsupported operation requested: FFFFFFF8DsCrackNames: Unsupported operation requested: FFFFFFF8Bad talloc magic value - unknown value
[2020/09/11 11:32:39.636346,  0] ../../lib/util/fault.c:132(smb_panic_default)
  smb_panic_default: PANIC (pid 368982): Bad talloc magic value - unknown value
[2020/09/11 11:32:39.682228,  0] ../../lib/util/fault.c:265(log_stack_trace)
  BACKTRACE: 64 stack frames:
   #0 /usr/local/samba/lib/libsamba-util.so.0(log_stack_trace+0x1f) [0x7f4298dccfe8]
   #1 /usr/local/samba/lib/libsamba-util.so.0(+0x1ad90) [0x7f4298dccd90]
   #2 /usr/local/samba/lib/libsamba-util.so.0(log_stack_trace+0) [0x7f4298dccfc9]
   #3 /usr/local/samba/lib/private/libtalloc.so.2(+0x29d2) [0x7f4299adf9d2]
   #4 /usr/local/samba/lib/private/libtalloc.so.2(+0x29f8) [0x7f4299adf9f8]
   #5 /usr/local/samba/lib/private/libtalloc.so.2(+0x2a76) [0x7f4299adfa76]
   #6 /usr/local/samba/lib/private/libtalloc.so.2(+0x46ad) [0x7f4299ae16ad]
   #7 /usr/local/samba/lib/private/libtalloc.so.2(talloc_check_name+0x33) [0x7f4299ae1747]
   #8 /usr/local/samba/lib/private/libdsdb-module-samba4.so(dsdb_module_am_system+0x3e) [0x7f4283e0920d]
   #9 /usr/local/samba/lib/ldb/acl.so(+0x7fe0) [0x7f4282101fe0]
   #10 /usr/local/samba/lib/private/libldb.so.2(ldb_next_request+0x14e) [0x7f42998adb13]
   #11 /usr/local/samba/lib/ldb/descriptor.so(+0x47d3) [0x7f42814d87d3]
   #12 /usr/local/samba/lib/private/libldb.so.2(ldb_next_request+0x14e) [0x7f42998adb13]
....

(In reply to William Ross from comment #8)
Yes, I think valgrind is what we need.

If that turns out to be too much overhead, compiling with Address Sanitiser is another option.

Thanks for your persistence in this!

*** Bug 14469 has been marked as a duplicate of this bug. ***

Update on this.
I was unable to use valgrind as enabling it on the DC made it slow slow users were immediately unable to access network resources.
I have been able to configure samba with address-sanitizer, but I ran into many issues getting it running (runtime errors regarding the order of libraries loading) so I configured a dedicated DC for this purpose. It runs, it often runs out of memory (currently configured with 16GB, it seems to use as much as I can give it) but it has never had the PANIC occur (the PANIC is occuring a couple of times per week on the 'primary' DC).
This may be because it is the secondary DC on this site so gets less traffic. Given its out of memory issues though I don't want to disable the other DC as users will notice issues.

My current potential solution, based on the issue perhaps being some corrupted records in AD, is to add a Windows DC to the domain, then demote and rebuild all of the Samba DCs from the Windows one. The hope being the Windows DC will spot the bad data and not write to to its own copy of AD.

Sorry to be the bearer of bad news, but I don't think laundering the records via a windows DC will be much help.

Windows does even less checking of existing records transmitted over DRS than Samba, so I don't think it will fix the records for you.

It is unfortunate that valgrind nor address-sanitizer (which needs a massive virtual address space) is working for you, those would tell us a lot.

My only request for now is if you could collect some more samples, perhaps something might jump out.  Also, if you turn on audit logging you might get some ideas which client is making the problem request, which might in turn help you reproduce it on demand.

https://wiki.samba.org/index.php/Setting_up_Audit_Logging

We would still like to get to the bottom of this.

To be clear, the samples would need to be of a debug build with gdb_backtrace configured, so we get clear line numbers.

(In reply to Andrew Bartlett from comment #13)

I think I know what the problem is and I'm working on a fix:

auth_info = dcesrv_call_session_info(dce_call); from
dcesrv_drsuapi_DsBind() is allocated on the scope of
the dce_call->conn, but the b_state (with ->sam_ctx)
is moved to dce_call->conn->assoc_group.

When the original connection leaves the assoc_group,
auth_info goes away, while b_state->sam_ctx can still
be used on a different connection.

I think this is only a DoS, where our prefork handling
restarts the helpers in supported versions.

Andrew, do you agree that we can just fix this without a security
release?

Currently we do DoS as a security release, but I've proposed changing this on our internal team list (currently stuck in moderation).

Created attachment 16718 [details]
WIP patches for master

We will do a security release for this as it is a use-after-free.

I'll get a CVE.

CVSSv3.1: AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:H (7.6)

Comment on attachment 16718 [details]
WIP patches for master

Looks good.  If it wasn't in a static function directly above the callers I would have made 'bool as_system' an enum per a good practice from Volker (I think). 

I notice you chose not to just throw in a talloc_reference() to solve it, which is probably for the best ;-)

Created attachment 16820 [details]
patch for master with RB+ tags (v01)

I've reviewed the patch, added my RB tags, CVE tags and confirmed this is otherwise the same as the applies cleanly to 4.15, 4.14 and 4.13 and am doing some CI on the backports.

As this is the same as attachment 16718 [details] (confirmed by diff of diff) I'm putting the CI+ marker on it.

Created attachment 16821 [details]
patch from master backported to 4.15 (v01)

Created attachment 16822 [details]
patch from master backported to 4.14 (v01)

Created attachment 16823 [details]
patch from master backported to 4.13 (v01)

William,

I'm preparing the security announcement text.  Can you let me know if you are OK to be credited as the reporter and any affiliation (eg company) you would like listed?

Thanks!

Andrew Bartlett

Created attachment 16824 [details]
advisory text (v01)

Comment on attachment 16824 [details]
advisory text (v01)

The advisory is missing the $VERSIONS and $REPORTER while we wait to work out when this will ship and get publication permission for the reporter.

Comment on attachment 16822 [details]
patch from master backported to 4.14 (v01)

The 4.14 and 4.13 patches will need a backport due to cmdline changes.

Hi Andrew,

Thanks to everyone for finding and fixing this (I was still failing to get a proper debug log for you).

Please accredit as:

William Ross, City West Country Ltd

Created attachment 16827 [details]
patch from master backported to 4.14 (v02)

Created attachment 16828 [details]
patch from master backported to 4.13

CI is running now on the 4.13 patch

Created attachment 16829 [details]
patch from master backported to 4.13 (v03)

This is under CI now.

Created attachment 16830 [details]
advisory text (v02)

(note the patch version numbers in the backports are not in sync, as each version has needed their own extra work).

I still think this is ready-to-go but I need the patches reviewed.  

If someone has the time to help out that would be awesome, and we can get this out the door.

Created attachment 16886 [details]
advisory test (v03)

New version of the advisory with slight reformatting, and a full-stop at the end.

Comment on attachment 16886 [details]
advisory test (v03)

Looks good.  We are pretty sure $VERSIONS will be 4.15.2, 4.14.10 and 4.13.14.

This is ready for the November 9th release...

G'Day Vendors,

This bug will also be part of the security release for Nov 9 2021.

But the patches on this bug are on their own independent from
the large combined patch on bug #14834.

This bug is only relevant as active directory domain controllers.

The release will happen around 18:00 UTC November 9th.

Created attachment 16971 [details]
patch from master backported to 4.12 (v01)

This patch applies on top of the v4.12 patch found at https://bugzilla.samba.org/show_bug.cgi?id=14725.

Created attachment 16975 [details]
patch from master backported to 4.10 (v01)

This patch applies on top of the v4.10 patch found at https://bugzilla.samba.org/show_bug.cgi?id=14725.

Comment on attachment 16971 [details]
patch from master backported to 4.12 (v01)

This patch is an exact backport of the 4.13 patch.  I've also read over it again and I'm happy with it.

Comment on attachment 16975 [details]
patch from master backported to 4.10 (v01)

This is a backport of the 4.13 patch with expected changes.  I'm happy with it.

This bug was referenced in samba v4-14-stable (Release samba-4.14.10):

a8fbaf0c96dddad39b8fc7a46fa5bb2fa6806f34
61c8272b27c533482b85dd902f4d237d0cc0ca89
5d212fb77f5336c25fc20571453d7b968bc5a6db
50c0ac89d52e4313d5c9cb502077df9062c6b7e6
258710a9f2145939d959a8512e0d40dfd32ef1b7
0200d5ab2f4beef5dd55657cf51e34319cd4f756
215fb2275f0feb1bdaec3148e5d24f649a716ad3
f583cda95abba596b4cd0201b9cfee97287d29c7
57959216435e32d3434edc0cd786f18aa139cf9c
b1aba4e2bc7946c7ef2f4de30f4a41b016bdab4e
25c944643f3d6ea55767a389423571a1136c68bc

This bug was referenced in samba v4-15-stable (Release samba-4.15.2):

4c59866c08e73532bd6aeb7374f83a98974de1cd
67b43eadd2bd78c4bc60eda75d5d4d5851e9afc1
6f971523a715822834a3152d1705b8afc0cc9a0d
129b3694a18feca4fcba5a5acbf0e6b201e928d5
462d635966e7eb87269d637b381ea43fa06fe49c
3b767f29f4c18b5d0da43a697712222444fcc3b3
091dd0fd5d7affe549b7c76c8209a2b8ea5b26e0
dbddd1cbcb169720c65a755ecf077d3727d63bb4
952ab2b82cd38969f7131721b3b7f542a65067dd
0b52f103889b3673e75ec7cd25356a3bf6267595
a87d07ccc56cfbd2ae3c061a9ce589838e8c4e90

This bug was referenced in samba v4-14-test:

a8fbaf0c96dddad39b8fc7a46fa5bb2fa6806f34
61c8272b27c533482b85dd902f4d237d0cc0ca89
5d212fb77f5336c25fc20571453d7b968bc5a6db
50c0ac89d52e4313d5c9cb502077df9062c6b7e6
258710a9f2145939d959a8512e0d40dfd32ef1b7
0200d5ab2f4beef5dd55657cf51e34319cd4f756
215fb2275f0feb1bdaec3148e5d24f649a716ad3
f583cda95abba596b4cd0201b9cfee97287d29c7
57959216435e32d3434edc0cd786f18aa139cf9c
b1aba4e2bc7946c7ef2f4de30f4a41b016bdab4e
25c944643f3d6ea55767a389423571a1136c68bc

This bug was referenced in samba v4-13-stable (Release samba-4.13.14):

f7636fb7215f83a5d8cc501ff46eed0954e10040
3db47b076d06e2f21f61a6fd1645b1ccb54869c6
ec1ea05e8f1dcca14ed7a92a0645da9ff73f33d3
5337dc5eaeb7abba3aeb73cc6ef0471ffc9e7bc8
6925a53a290d6d5fb310820bc62080492521cee4
7c3b037600077ade1d0ee97f5707e1c5061c1b28
061c125c6129634d220c1074fa8ed5eaa8b0e09c
caf3d32f68f91ea83c7f601577dd1f7c98f030e5
79d62d83e23fe5969cb432262ab9addad59a3b8d
08b6c8fda591c129adecd0779bf4a62386b8c740
0203330e2fa23482d99809e777ccb8a93a728aa3

This bug was referenced in samba v4-13-test:

f7636fb7215f83a5d8cc501ff46eed0954e10040
3db47b076d06e2f21f61a6fd1645b1ccb54869c6
ec1ea05e8f1dcca14ed7a92a0645da9ff73f33d3
5337dc5eaeb7abba3aeb73cc6ef0471ffc9e7bc8
6925a53a290d6d5fb310820bc62080492521cee4
7c3b037600077ade1d0ee97f5707e1c5061c1b28
061c125c6129634d220c1074fa8ed5eaa8b0e09c
caf3d32f68f91ea83c7f601577dd1f7c98f030e5
79d62d83e23fe5969cb432262ab9addad59a3b8d
08b6c8fda591c129adecd0779bf4a62386b8c740
0203330e2fa23482d99809e777ccb8a93a728aa3

This bug was referenced in samba v4-15-test:

4c59866c08e73532bd6aeb7374f83a98974de1cd
67b43eadd2bd78c4bc60eda75d5d4d5851e9afc1
6f971523a715822834a3152d1705b8afc0cc9a0d
129b3694a18feca4fcba5a5acbf0e6b201e928d5
462d635966e7eb87269d637b381ea43fa06fe49c
3b767f29f4c18b5d0da43a697712222444fcc3b3
091dd0fd5d7affe549b7c76c8209a2b8ea5b26e0
dbddd1cbcb169720c65a755ecf077d3727d63bb4
952ab2b82cd38969f7131721b3b7f542a65067dd
0b52f103889b3673e75ec7cd25356a3bf6267595
a87d07ccc56cfbd2ae3c061a9ce589838e8c4e90

This bug was referenced in samba master:

923c80eea96e725bdfc9e91f854f459bbaa8954f
73b6ed864e084814e0a39c1d16c6217ba0ca26dd
45315f2284d9971d0b9e63b61bfdeab5e9589b54
b9deab4ca43a2d08bed6950c05a57a7b2c7557bd
b173ac586a688c2c3c6e75b02952e939fd0d4698
897c0e8fc6fe9a9323f3ff657dc4245a7249c6fd
af6151ef122a4f452d486e541626c2a1feacb369
965fe0e906263bffd6fb994263e51a8435f155d5
2a159e6f036db497bd976e2d165db5c187a09cf6
5724868c22eb2ecd6d58fd167f315699ede53043
3121be69cac7748d1cb01273c0d09fab2fe726a0

The patches addressing this issue have been pushed to master and security releases made.

Removing vendor CC (so that any public comments don't need to be broadcast so widely) and opening these bugs to the public.  

These are the "other issues" part of the big release we just made, the remainder are private for a little longer.

If you wish to continue to be informed about any changes here please CC individually.