s3: Fix a winbind race leading to 100% CPU
authorVolker Lendecke <vl@samba.org>
Fri, 26 Aug 2011 14:54:18 +0000 (16:54 +0200)
committerMichael Adam <obnox@samba.org>
Tue, 11 Oct 2011 13:21:03 +0000 (15:21 +0200)
This fixes a race condition that leads to the winbindd_children list becoming
corrupted. It happens when on a busy winbind SIGCHLD is a bit late.

Imagine a winbind with multiple requests in the queue for a single child. Child
dies, and before the SIGCHLD handler is called we find the socket to be dead.
wb_child_request_done is called, receiving an error from wb_simple_trans_recv.
It closes the socket. Then immediately the wb_child_request_trigger will do
another fork_domain_child before the signal handler is called. This means that
we do another fork_domain_child, we have child->sock==-1 at this point.
fork_domain_child will do a DLIST_ADD(winbindd_children, child) a second time
where the child is already part of that list. This corrupts the list. Then the
signal handler kicks in, spinning in

for (child = winbindd_children; child != NULL; child = child->next) {

forever. Not good. This patch makes sure that both conditions (sock==-1 and not
part of the list) for a winbindd_child struct match up.

source3/winbindd/winbindd_dual.c

index 1078f8d374d32f734740b8eadc8071164a3fae4e..0b4cc6ef6e60fd04b20901bd43e2a67d3446d787 100644 (file)
@@ -43,6 +43,8 @@
 extern bool override_logfile;
 extern struct winbindd_methods cache_methods;
 
+static struct winbindd_child *winbindd_children = NULL;
+
 /* Read some data from a client connection */
 
 static NTSTATUS child_read_request(struct winbindd_cli_state *state)
@@ -171,6 +173,7 @@ static void wb_child_request_done(struct tevent_req *subreq)
                 */
                close(state->child->sock);
                state->child->sock = -1;
+               DLIST_REMOVE(winbindd_children, state->child);
                tevent_req_error(req, err);
                return;
        }
@@ -489,8 +492,6 @@ void setup_child(struct winbindd_domain *domain, struct winbindd_child *child,
        SMB_ASSERT(child->binding_handle != NULL);
 }
 
-static struct winbindd_child *winbindd_children = NULL;
-
 void winbind_child_died(pid_t pid)
 {
        struct winbindd_child *child;