[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: ldap_new_connection() mutex issue



> masarati@aero.polimi.it writes:
>> ldap_new_connection(), if ( connect ) and lconn_server->lud_exts contain
>> the tls ext, first unlocks and then re-locks ld_req_mutex and
>> ld_res_mutex.  As far as I understand, while the former is actually held
>> by the caller(s) of ldap_new_connection(), the latter is not.  If this
>> analysis is correct, I'll post an ITS.
>
> It is only relevant when the 'connect' argument is != 0: In
> request.c:ldap_send_server_request() and open.c:ldap_open_defconn().
>
> ldap_result() locks ldap_res_mutex, which is one road to the first
> case.  Hopefully all roads do something similar, I haven't checked.
>
> However, open.c doesn't lock either mutex:
>   /* Caller should hold the req_mutex if simultaneous accesses are
> possible */
>   int ldap_open_defconn( LDAP *ld )
> Looks like the "if" in the comment at best should have listed some more
> cases, to avoid code paths into using these mutexes.

OK, I've checked.

ldap_new_connection() must be called holding ld_req_mutex; ld_res_mutex
must be also held if connection != 0 or bind != NULL

ldap_new_connection() is called by ldap_send_server_request() with connect
= 1; this, in turn, is called holding ld_req_mutex but not ld_res_mutex

ldap_new_connection() is also called by:

- ldap_open_defconn() with connect = 1; this in turn must be called
holding ld_req_mutex

- ldap_init_fd() with connect = 0 and bind = NULL; this function does not
hold any mutex (nor any is required since the LDAP structure has just been
created and thus cannot be shared)

- ldap_open_internal_connection() (same as above; never used in OpenLDAP
code)

I guess we need to eliminate the unlock/lock of ld_res_mutex and inform
ldap_new_connection() about whether ld_req_mutex is locked or not...

p.