Bug 12505 - smbd will crash if keytab do not exist and SMB session is requested
smbd will crash if keytab do not exist and SMB session is requested
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other
All All
: P5 normal
: ---
Assigned To: Andrew Bartlett
Samba QA Contact
Depends on: 10490
  Show dependency treegraph
Reported: 2017-01-10 01:33 UTC by Jura Sasek
Modified: 2017-08-20 15:54 UTC (History)
2 users (show)

See Also:

fix (425 bytes, patch)
2017-01-10 01:33 UTC, Jura Sasek
no flags Details

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: