Bug 456372 - assert FD_ISSET unnecessary in function slapd_clr_write
assert FD_ISSET unnecessary in function slapd_clr_write
Status: CLOSED CURRENTRELEASE
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: openldap (Show other bugs)
4.6
All Linux
urgent Severity high
: rc
: ---
Assigned To: Jan Vcelak
: ZStream
Depends On:
Blocks: 510233
  Show dependency treegraph
 
Reported: 2008-07-23 03:48 EDT by Jatin Nansi
Modified: 2013-03-03 20:27 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-06-14 16:42:45 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
backported patch (986 bytes, patch)
2008-09-01 04:30 EDT, Jan Safranek
no flags Details | Diff

  None (edit)
Description Jatin Nansi 2008-07-23 03:48:55 EDT
Description of problem:
Customer is running ldap server using Openldap, The issue is slapd crashes with
a core file. Customer is running RHEL4 update 4, with openldap version
"openldap-2.2.13-8.el4_6.4-i386".

Provide time and date of the problem:
There is no specific time, it has happened 3 times from Octoboer 07. It just
happens randomly and not reproducible. 

The Backtrace of core file is below:

(gdb) bt
#0  0x00223c9e in raise () from /lib/tls/i686/libc.so.6
#1  0x002252a0 in abort () from /lib/tls/i686/libc.so.6
#2  0x0021d92c in __assert_fail () from /lib/tls/i686/libc.so.6
#3  0x008acb55 in slapd_clr_write (s=60, wake=0) at
../../../servers/slapd/daemon.c:308
#4  0x008b45af in connection_write (s=60) at
../../../servers/slapd/connection.c:1823
#5  0x008af45f in slapd_daemon_task (ptr=0x0) at
../../../servers/slapd/daemon.c:1890
#6  0x00dc3144 in start_thread () from /lib/tls/i686/libpthread.so.0
#7  0x002b7c8e in clone () from /lib/tls/i686/libc.so.6

There is an assertion failure at line 308 in slapd_clr_write().

Definition of slapd_clr_write():
    305 void slapd_clr_write(ber_socket_t s, int wake) {
    306         ldap_pvt_thread_mutex_lock( &slap_daemon.sd_mutex );
    307 
    308         assert( FD_ISSET( s, &slap_daemon.sd_actives) );
    309         FD_CLR( s, &slap_daemon.sd_writers );
    310 
    311         ldap_pvt_thread_mutex_unlock( &slap_daemon.sd_mutex );
    312         WAKE_LISTENER(wake);
    313 }

We found an similar bug in the upstream openldap ITS bug tracking system,
although the bug description applies to newer versions (2.3.27 in RHEL5) of
openldap:
http://www.openldap.org/its/index.cgi/Software%20Bugs?id=5489;expression=assert


Suggested resolution:
------------------------
From man FD_ISSET:
FD_ISSET(fd, fdsetp) shall evaluate to non-zero if the file descriptor fd is a
member of the set pointed to by fdsetp, and shall evaluate to zero otherwise.

From man FD_CLR:
FD_CLR(fd,  fdsetp)  shall  remove the file descriptor fd from the set pointed
to by fdsetp. If fd is not a member of this set, there shall be no effect on the
set, nor will an error be returned.

Since it does not matter to FD_CLR if fd is a member of the set pointed to by
fdsetp, I don't see any point in asserting the same. Maybe we should just log a
warning here?
Comment 1 Jatin Nansi 2008-07-23 04:10:35 EDT
Further looking at the code where slapd_daemon_task() calls connection_write():
the comment says that the stream may be inactive. In this case, we should not be
asserting to check if it is active:
--------------------------------------------------------

             /*
              * NOTE: it is possible that the connection was closed
              * and that the stream is now inactive.
              * connection_write() must valid the stream is still
              * active.
              */

              if ( connection_write( wd ) < 0 ) {
                    FD_CLR( (unsigned) wd, &readfds );
                    slapd_close( wd );
              }
--------------------------------------------------------
Comment 2 Jan Safranek 2008-07-24 06:51:16 EDT
upstream fix:
http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/daemon.c.diff?r1=1.418&r2=1.419&hideattic=1&sortbydate=0

It will need some backporting though. AFAIK there is no reproducer, right?

Openldap in RHEL5 suffers from the same bug, which will get fixed with rebase to
openldap-2.3.43 (bug #454994).
Comment 3 Jatin Nansi 2008-08-05 00:42:57 EDT
Jan,
You are correct, there is no reproducer.
Comment 7 Jan Safranek 2008-09-01 04:30:05 EDT
Created attachment 315458 [details]
backported patch
Comment 8 RHEL Product and Program Management 2008-10-31 12:42:35 EDT
This request was evaluated by Red Hat Product Management for
inclusion, but this component is not scheduled to be updated in
the current Red Hat Enterprise Linux release. If you would like
this request to be reviewed for the next minor release, ask your
support representative to set the next rhel-x.y flag to "?".

Note You need to log in before you can comment on or make changes to this bug.