Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 456372 - assert FD_ISSET unnecessary in function slapd_clr_write
Summary: assert FD_ISSET unnecessary in function slapd_clr_write
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: openldap
Version: 4.6
Hardware: All
OS: Linux
urgent
high
Target Milestone: rc
: ---
Assignee: Jan Vcelak
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 510233
TreeView+ depends on / blocked
 
Reported: 2008-07-23 07:48 UTC by Jatin Nansi
Modified: 2018-10-20 02:43 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-06-14 20:42:45 UTC


Attachments (Terms of Use)
backported patch (deleted)
2008-09-01 08:30 UTC, Jan Safranek
no flags Details | Diff

Description Jatin Nansi 2008-07-23 07:48:55 UTC
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 08:10:35 UTC
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 10:51:16 UTC
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 04:42:57 UTC
Jan,
You are correct, there is no reproducer.

Comment 7 Jan Safranek 2008-09-01 08:30:05 UTC
Created attachment 315458 [details]
backported patch

Comment 8 RHEL Product and Program Management 2008-10-31 16:42:35 UTC
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.