Bug 10280 - winbind panic if AD server is down
winbind panic if AD server is down
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: Winbind
4.0.9
IA64 Linux
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-19 13:29 UTC by Lev
Modified: 2014-07-17 18:55 UTC (History)
6 users (show)

See Also:


Attachments
winbindd log file (10.73 KB, application/octet-stream)
2013-11-19 13:29 UTC, Lev
no flags Details
Patch (951 bytes, patch)
2013-11-19 21:07 UTC, Christian Ambach
no flags Details
Patches for 4.0 and 4.1 with cherry-pick information (1.24 KB, patch)
2014-01-02 18:45 UTC, Christian Ambach
metze: review+
Details
git-am fix for master, 4.1.next and 4.0.next. (1.17 KB, patch)
2014-07-02 03:35 UTC, Jeremy Allison
ambi: review+
jra: review? (metze)
obnox: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lev 2013-11-19 13:29:28 UTC
Created attachment 9441 [details]
winbindd log file

I stopped AD server and then I tried to change the security of a file onto the SMB share. winbindd immediately paniced. In gdb I see panic is in a function rids_to_names(), it tries to assign to (*types)[i], while *types==NULL. Looks like *names and *types remain uninitialized in domain->backend->rids_to_names() when it returns NT_STATUS_IO_TIMEOUT.

(gdb) bt
#0  0x00007f9a8f2ae425 in __open_catalog () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007f9a912a2bea in smb_panic_s3 (why=0x7f9a960ec2dd "internal error") at ../source3/lib/util.c:833
#2  0x00007f9a960d8ef8 in smb_panic (why=0x7f9a960ec2dd "internal error") at ../lib/util/fault.c:159
#3  0x00007f9a960d8bdb in fault_report (sig=11) at ../lib/util/fault.c:77
#4  0x00007f9a960d8bf0 in sig_fault (sig=11) at ../lib/util/fault.c:88
#5  <signal handler called>
#6  0x00007f9a9697b918 in rids_to_names (domain=0x7f9a96fdbcf0, mem_ctx=0x7f9a96fdb090, domain_sid=0x7f9a96ffb2e0, rids=0x7f9a96fe8cf0, num_rids=2, domain_name=0x7fff384dd0c8, names=0x7fff384dd0d0, types=0x7fff384dd0d8)
    at ../source3/winbindd/winbindd_cache.c:2141
#7  0x00007f9a969a8f1f in _wbint_LookupRids (p=0x7fff384dd1a0, r=0x7f9a96ff3080) at ../source3/winbindd/winbindd_dual_srv.c:533
#8  0x00007f9a969cc4c3 in api_wbint_LookupRids (p=0x7fff384dd1a0) at default/source3/librpc/gen_ndr/srv_wbint.c:1437
#9  0x00007f9a969a7a31 in winbindd_dual_ndrcmd (domain=0x7f9a96fdbcf0, state=0x7fff384dd340) at ../source3/winbindd/winbindd_dual_ndr.c:322
#10 0x00007f9a969a3444 in child_process_request (child=0x7f9a96fcbbe0, state=0x7fff384dd340) at ../source3/winbindd/winbindd_dual.c:441
#11 0x00007f9a969a6e64 in fork_domain_child (child=0x7f9a96fcbbe0) at ../source3/winbindd/winbindd_dual.c:1554
#12 0x00007f9a969a2516 in wb_child_request_trigger (req=0x7f9a96fdb8f0, private_data=0x0) at ../source3/winbindd/winbindd_dual.c:146
#13 0x00007f9a95460189 in tevent_queue_immediate_trigger (ev=0x7f9a96fcc210, im=0x7f9a96fdc610, private_data=0x7f9a96fdc580) at ../lib/tevent/tevent_queue.c:144
#14 0x00007f9a9545fe28 in tevent_common_loop_immediate (ev=0x7f9a96fcc210) at ../lib/tevent/tevent_immediate.c:135
#15 0x00007f9a912bf32b in run_events_poll (ev=0x7f9a96fcc210, pollrtn=0, pfds=0x0, num_pfds=0) at ../source3/lib/events.c:192
#16 0x00007f9a912bf9d3 in s3_event_loop_once (ev=0x7f9a96fcc210, location=0x7f9a96a0f770 "../source3/winbindd/winbindd.c:1525") at ../source3/lib/events.c:303
#17 0x00007f9a9545ef3f in _tevent_loop_once (ev=0x7f9a96fcc210, location=0x7f9a96a0f770 "../source3/winbindd/winbindd.c:1525") at ../lib/tevent/tevent.c:530
#18 0x00007f9a9697192a in main (argc=1, argv=0x7fff384def48, envp=0x7fff384def58) at ../source3/winbindd/winbindd.c:1525
(gdb) f 6
#6  0x00007f9a9697b918 in rids_to_names (domain=0x7f9a96fdbcf0, mem_ctx=0x7f9a96fdb090, domain_sid=0x7f9a96ffb2e0, rids=0x7f9a96fe8cf0, num_rids=2, domain_name=0x7fff384dd0c8, names=0x7fff384dd0d0, types=0x7fff384dd0d8)
    at ../source3/winbindd/winbindd_cache.c:2141
2141                                    (*types)[i] = SID_NAME_UNKNOWN;
(gdb) p *types
$1 = (enum lsa_SidType *) 0x0
(gdb) p/x result
$2 = {v = 0xc00000b5}
(gdb)
Comment 1 Christian Ambach 2013-11-19 21:07:45 UTC
Created attachment 9447 [details]
Patch

Does this patch help?
Comment 2 Christian Ambach 2014-01-02 18:45:07 UTC
Created attachment 9553 [details]
Patches for 4.0 and 4.1 with cherry-pick information

Patch that went into master
Comment 3 Karolin Seeger 2014-01-02 18:58:55 UTC
Pushed to autobuild-v4-1-test, v4-0-test is already frozen.
Comment 4 Karolin Seeger 2014-01-04 18:05:23 UTC
Pushed to v4-1-test.
Comment 5 Karolin Seeger 2014-01-07 08:36:51 UTC
Pushed to autobuild-v4-0-test.
Comment 6 Karolin Seeger 2014-01-10 08:13:07 UTC
(In reply to comment #5)
> Pushed to autobuild-v4-0-test.

Pushed to v4-0-test.
Closing out bug report.

Thanks!
Comment 7 redhaze1 2014-07-01 01:46:16 UTC
(In reply to comment #2)
> Created attachment 9553 [details]
> Patches for 4.0 and 4.1 with cherry-pick information
> 
> Patch that went into master

There appears to be a problem with this patch. The comparisons to NULL seem the wrong way around, returning a NT_STATUS_NO_MEMORY error if the talloc_array() succeeds.
Comment 8 Warren 2014-07-01 01:55:03 UTC
(In reply to comment #7)
> (In reply to comment #2)
> > Created attachment 9553 [details] [details]
> > Patches for 4.0 and 4.1 with cherry-pick information
> > 
> > Patch that went into master
> 
> There appears to be a problem with this patch. The comparisons to NULL seem the
> wrong way around, returning a NT_STATUS_NO_MEMORY error if the talloc_array()
> succeeds.

Yeah, it looks wrong to me too. It is also worth to mention this bug was seen in 3.6 (at least in 3.6.12) in case someone want to apply this bug fix there.
Comment 9 Jeremy Allison 2014-07-01 18:23:02 UTC
Patch for 4.0.x and 4.1.x is bad ! Karolin, I will fix this - then you can re-push.

Jeremy.
Comment 10 Jeremy Allison 2014-07-02 03:35:35 UTC
Created attachment 10065 [details]
git-am fix for master, 4.1.next and 4.0.next.

This additional patch needs to be added to fix the problem.

Metze, Christian, please review otherwise winbindd will be broken.

Thanks,

Jeremy.
Comment 11 Michael Adam 2014-07-02 05:41:17 UTC
Comment on attachment 10065 [details]
git-am fix for master, 4.1.next and 4.0.next.

Obvious Goodnees. Gonna pus to master.
Comment 12 Michael Adam 2014-07-02 05:54:42 UTC
it's in autobuild.
also did a minor update to the commit msg to include
"BUG: " before the bso link (as someone told me was our
policy..)

Let's wait for them to hit master and redo the
patch for 4.1/0 w/ cherry-pick info

Michael
Comment 13 Christian Ambach 2014-07-02 12:42:53 UTC
Comment on attachment 10065 [details]
git-am fix for master, 4.1.next and 4.0.next.

OMG.. sorry for this stupid error
Comment 14 Jeremy Allison 2014-07-02 16:05:45 UTC
(In reply to comment #13)
> Comment on attachment 10065 [details]
> git-am fix for master, 4.1.next and 4.0.next.
> 
> OMG.. sorry for this stupid error

Christian, don't worry about it. It got reviewed so it's not all your fault :-).

Humans make mistakes (especially in C), I've added my share of security bugs over the years, and those are *much* worse :-).

Jeremy.
Comment 15 Karolin Seeger 2014-07-07 07:50:57 UTC
(In reply to comment #9)
> Patch for 4.0.x and 4.1.x is bad ! Karolin, I will fix this - then you can
> re-push.
> 
> Jeremy.

Pushed additional Patch to autobuild-v4-[0|1]-test.
Comment 16 Karolin Seeger 2014-07-17 18:55:03 UTC
(In reply to comment #15)
> (In reply to comment #9)
> > Patch for 4.0.x and 4.1.x is bad ! Karolin, I will fix this - then you can
> > re-push.
> > 
> > Jeremy.
> 
> Pushed additional Patch to autobuild-v4-[0|1]-test.

Pushed to both branches.
Closing out bug report.

Thanks!