Created attachment 4214 [details]
Patch for 3.3 and 3.2

Karolin, this is a fix Isilon added for 3.4 and master that is an identical bug in 3.2 and 3.3. Due to the original code being uninitialized memory this bug can cause random segfaults, so this patch is important for the next 3.2 and 3.3 releases.

Please review.

Thanks,

Jeremy.

After a few days into a stress test, the following backtrace was produced.  The
commit hash in comment #0 fixes this.

#0  0x28976e37 in kill () from /lib/libc.so.6
#1  0x28976dd4 in raise () from /lib/libc.so.6
#2  0x28975ae8 in abort () from /lib/libc.so.6
#3  0x082a6612 in dump_core () at lib/fault.c:331
#4  0x082b57dd in smb_panic (why=0x859650f "internal error") at lib/util.c:1496
#5  0x082a5f8c in sig_fault (sig=11) at lib/fault.c:47
#6  <signal handler called>
#7  0x08289b6b in talloc_strdup (t=0x863aab0, 
    p=0x69637361 <Address 0x69637361 out of bounds>)
    at ../lib/talloc/talloc.c:1355
#8  0x0826836a in lookup_sids (mem_ctx=0x0, num_sids=1, sids=0x7f7fdf64, 
    level=1, ret_domains=0x0, ret_names=0x0) at passdb/lookup_sid.c:913
#9  0x08268efb in lookup_sid (mem_ctx=0x863a9f0, sid=0x8646930, 
    ret_domain=0x0, ret_name=0x0, ret_type=0x7f7fdf88)
    at passdb/lookup_sid.c:957
#10 0x0825bdff in pdb_get_group_sid (sampass=0x8650a30)
    at passdb/pdb_get_set.c:225
#11 0x08266f64 in pdb_get_group_rid (sampass=0x69637361)
    at passdb/pdb_compat.c:45
#12 0x08261e7b in init_buffer_from_samu (buf=0x7f7fe0a8, sampass=0x8650a30, 
    size_only=false) at passdb/passdb.c:1763
#13 0x0826257e in pdb_copy_sam_account (dst=0x863c430, src=0x8650a30)
    at passdb/passdb.c:2027
#14 0x082f8e17 in copy_serverinfo (mem_ctx=0x0, src=0x864fab0)
    at auth/auth_util.c:1385
#15 0x082f915d in make_server_info_guest (mem_ctx=0x0, server_info=0x69637361)
    at auth/auth_util.c:1435
#16 0x082f6382 in check_guest_security (auth_context=0x8646530, 
    my_private_data=0x0, mem_ctx=0x863a8f0, user_info=0x0, 
    server_info=0x863a578) at auth/auth_builtin.c:45
#17 0x082f1384 in check_ntlm_password (auth_context=0x8646530, 
    user_info=0x8645d80, server_info=0x863a578) at auth/auth.c:258
#18 0x082fc7c5 in auth_ntlmssp_check_password (ntlmssp_state=0x0, 
    user_session_key=0xffffffff, lm_session_key=0xffffffff)
    at auth/auth_ntlmssp.c:116
#19 0x0814d642 in ntlmssp_server_auth (ntlmssp_state=0x8648030, request=
      {data = 0x8635630 "NTLMSSP", length = 95}, reply=0xffffffff)
    at libsmb/ntlmssp.c:797
#20 0x0814ce02 in ntlmssp_update (ntlmssp_state=0x8648030, in=
      {data = 0x69637361 <Address 0x69637361 out of bounds>, length = 140708248}, out=0x7f7fe390) at libsmb/ntlmssp.c:342
#21 0x082fcb6d in auth_ntlmssp_update (auth_ntlmssp_state=0x0, request=
      {data = 0x8635630 "NTLMSSP", length = 95}, reply=0x7f7fe390)
    at auth/auth_ntlmssp.c:215
#22 0x080e808b in reply_sesssetup_and_X_spnego (req=0x86561d0)
    at smbd/sesssetup.c:933
#23 0x080e8ed2 in reply_sesssetup_and_X (req=0x86561d0)
    at smbd/sesssetup.c:1433
#24 0x08116b7e in switch_message (type=115 's', req=0x86561d0, size=298)
    at smbd/process.c:1356
#25 0x08116ee0 in process_smb (conn=0x863a430, inbuf=0x86561d0 "sv\a%G���%@\200", 
    nread=298, unread_bytes=0, encrypted=false, deferred_pcd=0x0)
    at smbd/process.c:1387
#26 0x08117907 in smbd_server_connection_handler (ev=0x86382b0, fde=0x8645bb0, 
    flags=0, private_data=0x863a430) at smbd/process.c:1860
#27 0x082c3b29 in run_events (ev=0x86382b0, selrtn=1, read_fds=0x7f7fe830, 
    write_fds=0x7f7fe7b0) at lib/events.c:126
#28 0x081185ea in smbd_process () at smbd/process.c:798
#29 0x0850ae53 in smbd_accept_connection (ev=0x86382b0, fde=0x86521b0, 
    flags=1, private_data=0x8652130) at smbd/server.c:388
#30 0x082c3b29 in run_events (ev=0x86382b0, selrtn=1, read_fds=0x7f7feae0, 
    write_fds=0x7f7fea60) at lib/events.c:126
#31 0x082c3d0b in s3_event_loop_once (ev=0x86382b0, 
    location=0x861f00c "smbd/server.c:662") at lib/events.c:185
#32 0x082c46d4 in _tevent_loop_once (ev=0x86382b0, 
    location=0x861f00c "smbd/server.c:662") at ../lib/tevent/tevent.c:478
#33 0x0850bf7d in main (argc=2, argv=0x7f7fee98) at smbd/server.c:662

The names array, instead of containing pointers to integers, contains the 
string "ascii" and the string "g-1" (the end of the node's hostname.)

I'm stunned that this did not show up earlier. To make sure I understand the patch right: The TALLOC_ZERO_ARRAY is not strictly necessary if everyone correctly looks at types[i]!=SID_NAME_UNKNOWN before looking at names[i], but it's just safe programming?

This patch is certainly right and worth putting into 3.2 and 3.3 pretty quickly.

Volker

Pushed. Will be included in 3.2.12 and 3.3.5.
Closing out bug report.

Thanks!