Mark Phalan wrote: > On 29 Mar 2010, at 20:20, David Schwartz <[email protected]> wrote: > > Mark Phalan wrote: > > > > I think this fix is actually a bad one. > I'm still not clear why you think that.
Because it doesn't solve the problem case -- where one library user sets callbacks that another library user isn't happy with. It won't help existing code, because it will already set callbacks (or it's hopelessly broken). It might help new code, but if we're going to help new code, we should do it a better way. > Even without the proposed change libraries should at least check > before clobbering the existing callbacks. Checking before setting will > be racy but still better than blindly replacing callbacks. I don't get what is this race you see. Are you talking about libraries that unset the OpenSSL callbacks while another library running in the same application might be using OpenSSL? That would still be fatal, and can never be made to work anyway. > The proposal doesn't make anything worse it 1) removes the race if > libraries set the callbacks properly How is that? I don't see how it removes the race. There are two cases for existing code: 1) They all set callbacks that are 100% compatible -- in which case there never was any race to fix. 2) Some of them set callbacks that are not 100% compatible with the callbacks others set -- in this case, there's still a race. > and 2) fixes MT issues if they > don't. If they don't set locking callbacks where such is clearly documented to be required, they're so hopelessly broken there's no point in trying to fix them in OpenSSL. What other requirements do they ignore? > Of course there is always the problem of what happens on dlclose() of > the lib if the callbacks are set... You can't shutdown a library while another library in that same application is or might be using it. It will never be possible to make that work. Again, that would be such hopelessly broken behavior that there's no point in trying to fix it for existing libraries/applications. A sensible way to fix it for new applications/libraries might make sense. > > The issue is that existing code may set the locking callbacks badly > > and the > > horse has already left the stable (we can't redesign them). > We can't easily get all the libraries which might end up eventually > loading OpenSSL to change their APIs so that locking callbacks can be > set but I think it wouldn't be too difficult to get them to do a check > for existing callbacks before doing the set. > Libraries which don't set the callbacks immediately become MT safe (as > far as OpenSSL calls are concerned). For new implementations, then best solution is an OpenSSL wrapper that can arbitrate when OpenSSL needs to be loaded and unloaded and who is using it. This could certainly be built into OpenSSL. But I don't think it would help existing applications unless they at least make some kind of 'add OpenSSL user' and 'remove OpenSSL user' kind of call. > > I don't see how > > this helps in that case -- the existing code will continue to set the > > locking callbacks badly, overriding the sane default. > Indeed. Until those are changed they will be broken both now and > after the proposed change. The proposal offers a sane way out. If we're talking about fixing it for new code, I think either an OpenSSL wrapper library, OpenSSL wrapper support in the application, or an 'add OpenSSL user'/'remove OpenSSL user' function in OpenSSL is the right way to go. DS ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [email protected] Automated List Manager [email protected]
