On Sun, Feb 10, 2008 at 10:06:14AM -0800, Russ Allbery wrote: > > Ok, I've scaled back the patch a bit before committing it because a > > deeper search leaves me uncertain that res_query and dn_expand are > > thread-safe even in current versions of glibc. Dropping the mutex for > > getaddrinfo() and getnameinfo() is sufficient to fix this bug, in any > > case.
> I have now discussed this and the related fact that we're using libldap_r > for ldapsearch (which from upstream's perspective is the actual problem) > with upstream. Upstream's stance on this is: > * Using libldap_r for anything other than slapd is flatly unsupported and > considered a bug. We should not be doing that. We should be treating > libldap_r as a private library only for slapd. > * libldap has no supported thread-safe API. Threaded programs that link > against libldap are required to handle locking themselves. Wow, ok, I'm very not happy with this answer. I think that *all* libraries should have thread-safe APIs; and aside from this minor blemish, libldap_r appears to already be a perfectly good thread-safe, reentrant library, supported by upstream or not. We have the following callers using libldap in Debian which also have need of thread-safety and currently take no additional steps to ensure libldap is thread-safe: - libpq5 - libnss-ldapd - evolution-data-server - libexchange-storage1.2-3 - libcurl3 - kdepimlibs5 - claws-mail (for bonus points, this app launches each LDAP query in a separate thread... :) - strongswan - libnss-ldap - libpam-ldap - python-ldap - openoffice.org-core - libsasl2-modules-ldap - libpam-smbpass - libsmbclient - freeradius-ldap (uses a mutex to ensure any given LDAP connection is only used by one thread at a time, but otherwise assumes that multiple LDAP connections can be going at once) ... and probably others besides, the list of reverse-depends is long and I haven't looked at them all. Moving the locking up the stack when we already have a perfectly thread-safe libldap available would be a significant regression. > * The root underlying problem would then be trying to use libnss-ldap and > slapd together on the same system at the same time, because libnss-ldap > pulls libldap into slapd's namespace. Upstream's opinion is that > libnss-ldap is broken and this regard and libnss-ldapd may be better. This particular bug is not related to using libnss-ldap and slapd together on the same system; it only requires one to try using an LDAP *client* on a system that uses nss_ldap for hosts resolution. The concerns about symbol collisions between libldap and libldap_r are specific to slapd only if we concede that libldap_r should be treated as a private library not intended for public consumption, which I am not because the non-threaded libldap is not an adequate replacement for a number of applications without duplicating a lot of work that's already been done centrally. > * People really shouldn't put hosts into LDAP; LDAP is a heavy-weight > protocol that is not suited for use as a DNS resolver. I happen to agree with that, but nss_ldap nevertheless implements all of the NSS services, and supporting the hosts implementation for glibc-based systems is a trivial change. Eliminating a lock that we *know* we don't need is hardly a bad thing, anyway. > The last we can communicate back to the user and perhaps even put into the > documentation for libnss-ldap and libnss-ldapd. For the rest, here is the > outline of an upstream-acceptable solution, which I'd love to be able to > get at. > * Revert the change to link everything against libldap_r and ship only > libldap in the libldap package (which will require nasty transition > stuff, but putting that side for right now). Adjust the shlibs in the > package accordingly, of course. Frankly, I would rather continue to ship libldap_r publically, even without upstream's blessing for this configuration, than inflict threading problems on the reverse-dependencies. We *know* that libldap_r is thread-safe because slapd itself exercises it in the "supported" configuration, and we already have to deal with frequently-changing libldap sonames primarily because of the ABI changes that are specific to the internal ldap_pvt_* functions - we ought to get some benefit for the migration troubles that's going to cause. > * Ship libldap_r in the slapd package or in a separate package referenced > by slapd and clearly document in the README.Debian for that package > that those libraries are intended for use only with slapd and any other > use is not supported. If that's really upstream's position, then they've been doing this wrong from the start. If libldap_r isn't supposed to be public, then they should have been building it as a convenience library and linking it statically into slapd; they should have kept all of the ldap_pvt_*/ldap_int_* functions *private*, instead of exporting them in libldap; and they should be maintaining a stable ABI for what is a pretty major system library instead of forcing rebuilds for every slapd internal ABI change. I'm dismayed. All of the problems with using libldap_r as the system libldap would be fixable over time, but it becomes considerably less feasible without upstream's support of the effort. Was your discussion with upstream somewhere public? -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developer http://www.debian.org/ [EMAIL PROTECTED] [EMAIL PROTECTED] -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]