Bug 12505 - smbd will crash if keytab do not exist and SMB session is requested
Summary: smbd will crash if keytab do not exist and SMB session is requested
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other (show other bugs)
Version: 4.4.8
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
Depends on: 10490
  Show dependency treegraph
Reported: 2017-01-10 01:33 UTC by Jura Sasek
Modified: 2021-03-11 11:47 UTC (History)
3 users (show)

See Also:

fix (425 bytes, patch)
2017-01-10 01:33 UTC, Jura Sasek
no flags Details
patch cherry-picked from master (1.12 KB, patch)
2021-02-10 00:57 UTC, Andrew Bartlett
jra: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Jura Sasek 2017-01-10 01:33:35 UTC
Created attachment 12808 [details]

smbd dump core on krb5_storage_free(0)

core 'core' of 1856:    /usr/sbin/smbd -D
 f612e6fc __lwp_sigqueue (6, ffffffef, ffffffec, fc32a718, 5, 6) + 8
 f60aa26c abort    (802c, f6178270, 6, 1, f6179b98, 0) + 108
 f4d70254 smb_panic_s3 (822170, f5b562f4, fff933d0, f4dc8000, f4d5b510,
f5b562f4) + 264
 f5fb3ed8 sig_fault (1, f5fe0000, f4d6fff0, c000, 43000, c008) + 3f8
 f6129c1c __sighndlr (b, 0, fc32ac00, f5fb3ae0, 0, f6178270) + c
 f611c8f4 call_user_handler (0, 0, b, f617d000, f5932a40, fc32ac00) + 370
 f611cbfc sigacthandler (b, 0, fc32ac00, 0, 0, f6178270) + 58
 --- called from signal handler with signal 11 (SIGSEGV) ---
 f50f0df4 krb5_storage_free (0, 0, 1c, 1, f6182ae0, 8) + 4
 f50d4cc4 fkt_end_seq_get (823338, 820270, fc32b050, fc32b074, fc32b050,
f50d4cc0) + 4
 f519297c fill_mem_keytab_from_system_keytab (823338, fc32b07c, ffffffff,
fc32b028, fc32b07c, fc32b05c) + 8bc
 f5192f1c gse_krb5_get_server_keytab (823338, 822120, 0, fffca900, f51be000,
35400) + 30c
 f5193c04 gse_init_server (ffff, 12, 0, 0, fc32b43c, 8220f0) + 1e4
because of the null pointer redirection:
krb5_storage_free(krb5_storage *sp)
    if(sp == NULL) return -1; <- this test should be added to avoid it off
          ^^^here smbd crashes (in library private/libkrb5-samba4.so.26.0.0)

...in file samba-4.4.8/source4/heimdal/lib/krb5/store.c
Comment 1 Jura Sasek 2017-08-12 21:45:47 UTC
Not yet fixed in trunk. What is the problem to add a check for null pointer to avoid of possible null pointer reference?
Comment 2 Andrew Bartlett 2017-08-14 09:16:56 UTC
(In reply to Jura Sasek from comment #1)
Can you get me the gdb 'bt full' backtrace?

In any case:

To fix it as proposed we first need to get such a fix upstream in Heimdal, or show it is already addressed there.  (Otherwise we risk loss of that fix if we ever manage an update). 

However, it might be that we just need to fix Samba not to free NULL.
Comment 3 Stefan Metzmacher 2017-08-14 09:34:29 UTC
(In reply to Jura Sasek from comment #1)

I'd guess that the fix for bug #10490 fixed this...
Can you try to verify that?

Comment 4 Paul Wise 2017-08-20 15:16:01 UTC
At my workplace we found a similar crash, but in winbindd when using AD authentication for SSH logins.

I note that #11824 is a similar bug report and probably a duplicate of this one.

I note that the NULL pointer dereference is already fixed in heimdal upstream, so the fix should be to sync samba's version of heimdal with the upstream heimdal, or enhance heimdal upstream so samba's version is no longer needed.

I note that the heimdal upstream fix returns 0 instead of -1 so it would probably be best to change heimdal upstream to return -1 instead.
Comment 5 Paul Wise 2017-08-20 15:50:09 UTC
I've verified that the heimdal upstream fix (similar to the one attached to this bug report, but returning 0 instead of -1) has fixed the crash that my workplace was experiencing.
Comment 6 Paul Wise 2017-08-20 15:54:26 UTC
I've filed a bug about removing or updating the samba copy of heimdal:

Comment 7 Paul Wise 2021-01-29 05:26:58 UTC
For Debian bullseye, this is the only remaining patch we are carrying at work, so it would be nice to have it backported to 4.13.x and or any other earlier releases that are still supported by samba upstream.
Comment 8 Andrew Bartlett 2021-01-29 05:57:47 UTC


Ideally find the change in upstream Heimdal and create a commit referencing that.
Comment 10 Samba QA Contact 2021-02-09 04:17:04 UTC
This bug was referenced in samba master:

Comment 11 Andrew Bartlett 2021-02-10 00:57:52 UTC
Created attachment 16440 [details]
patch cherry-picked from master

Patch for 4.12, 4.13, 4.14 (and all other releases really, this hasn't changed in ages).
Comment 12 Jeremy Allison 2021-02-10 17:19:43 UTC
Comment on attachment 16440 [details]
patch cherry-picked from master

Comment 13 Jeremy Allison 2021-02-10 17:20:19 UTC
Re-assigning to Karolin for inclusion in all supported releases.
Comment 14 Karolin Seeger 2021-02-15 12:29:46 UTC
(In reply to Jeremy Allison from comment #13)
Pushed to autobuild-v4-{14,13,12}-test.
Comment 15 Samba QA Contact 2021-02-16 17:17:05 UTC
This bug was referenced in samba v4-13-test:

Comment 16 Samba QA Contact 2021-02-16 18:28:13 UTC
This bug was referenced in samba v4-14-test:

Comment 17 Samba QA Contact 2021-02-16 22:34:14 UTC
This bug was referenced in samba v4-12-test:

Comment 18 Samba QA Contact 2021-02-18 09:52:56 UTC
This bug was referenced in samba v4-14-stable (Release samba-4.14.0rc3):

Comment 19 Samba QA Contact 2021-03-09 11:00:39 UTC
This bug was referenced in samba v4-13-stable (Release samba-4.13.5):

Comment 20 Samba QA Contact 2021-03-11 11:47:04 UTC
This bug was referenced in samba v4-12-stable (Release samba-4.12.12):