Fix a race condition in winbind leading to a panic
authorVolker Lendecke <vl@samba.org>
Sun, 24 May 2009 16:57:13 +0000 (18:57 +0200)
committerKarolin Seeger <kseeger@samba.org>
Tue, 2 Jun 2009 10:41:57 +0000 (12:41 +0200)
In winbind, we do multiple events in one select round. This needs fixing, but
as long as we're still using it, for efficiency reasons we need to do that.

What can happen is the following: We have outgoing data pending for a client,
thus

state->fd_event.flags == EVENT_FD_WRITE

Now a new client comes in, we go through the list of clients to find an idle
one. The detection for idle clients in remove_idle_client does not take the
pending data into account. We close the socket that has pending outgoing data,
the accept(2) one syscall later gives us the same socket.

In new_connection(), we do a setup_async_read, setting up a read fde. The
select from before however had found the socket (that we had already closed!!)
to be writable. In rw_callback we only want to see a readable flag, and we
panic in the SMB_ASSERT(flags == EVENT_FD_READ).

Found using

bin/smbtorture //127.0.0.1/tmp -U% -N 500 -o 2 local-wbclient

Volker

(commit 68c5c6df in master)
(cherry picked from commit d12681489f18df97b11c4ce6e069d6e2d006c184)

source/winbindd/winbindd.c

index 683e55401ddcb1c2f79f0d3f72d75f80aa7c30a8..929a4ea95dd87cee0c023721f8fd2317c799a3e7 100644 (file)
@@ -798,6 +798,7 @@ static bool remove_idle_client(void)
 
        for (state = winbindd_client_list(); state; state = state->next) {
                if (state->response.result != WINBINDD_PENDING &&
+                   state->fd_event.flags == EVENT_FD_READ &&
                    !state->getpwent_state && !state->getgrent_state) {
                        nidle++;
                        if (!last_access || state->last_access < last_access) {