Page MenuHomeFreeBSD

Avoid symbol clashes between libmd and libcrypto
ClosedPublic

Authored by thomas on Apr 3 2015, 8:44 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 29 2024, 1:15 AM
Unknown Object (File)
Jan 23 2024, 10:45 PM
Unknown Object (File)
Dec 27 2023, 10:56 AM
Unknown Object (File)
Dec 27 2023, 10:55 AM
Unknown Object (File)
Dec 19 2023, 4:15 PM
Unknown Object (File)
Dec 19 2023, 4:15 PM
Unknown Object (File)
Dec 10 2023, 5:51 AM
Unknown Object (File)
Oct 2 2023, 9:31 PM

Details

Summary

This change ensures that libmd does not define any symbol that is also present in libcrypto, providing a fix for PR 199119.

Test Plan

Tested by rebuilding and reinstalling libmd, rebuilding and reinstalling crontab(1), and observing that it did not hang anymore in a context where nss_ldap is used and connects to an LDAP server using SSL.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

thomas retitled this revision from to Avoid symbol clashes between libmd and libcrypto.
thomas updated this object.
thomas edited the test plan for this revision. (Show Details)
roberto edited edge metadata.

Looks reasonable to me.

This revision is now accepted and ready to land.Apr 3 2015, 9:50 AM

Looks reasonable to me; I use the same approach in the hash code I have in libcperciva. Is there a reason you're not doing this to md4 and md5 as well though?

I deliberately prefixed only those specific symbols that do clash with libcrypto. But indeed we could just as well consistently prefix all symbols in libmd.

thomas edited edge metadata.

Alternate version prefixing all symbols in libmd.

This revision now requires review to proceed.Apr 4 2015, 7:19 AM
bapt added a reviewer: bapt.
bapt added a subscriber: bapt.

I was exactly looking into doing that ! that will allow to modify things in base to always only depend on libmd like libarchive (making it easier have the ports tree use only openssl from ports and/or libressl from ports)!

This revision is now accepted and ready to land.Apr 4 2015, 12:47 PM

@cperciva @roberto would you mind commenting on the second version (updated to uniformly prefix all symbols in libmd, rather than just those that actually, currently conflict with libcrypto)?

Thanks!
Thomas.

roberto edited edge metadata.

Still OK with it, thanks.

delphij added a reviewer: delphij.
delphij added a subscriber: delphij.

Hi,

I got bite by this and used rS281372 to workaround it. Are you going to commit this anytime soon?

A second thought -- I wonder if we could use versioned symbols to achieve the same goal?

delphij removed a reviewer: delphij.

Not entirely clear to me how symbol versioning would help here. libmd provides just one version of these symbols, and we need to prevent any reference to the same-named symbols in libcrypto from getting (erroneously) resolved to the libmd version.

Version symbols will only help if openssl also have version symbol

In D2216#29, @thomas wrote:

Not entirely clear to me how symbol versioning would help here. libmd provides just one version of these symbols, and we need to prevent any reference to the same-named symbols in libcrypto from getting (erroneously) resolved to the libmd version.

I think @bapt was right that symbol versioning will not help (and it does not solve the conflict issue anyway).

BTW. @ume have pointed D1542 to me which uses __weak_reference to create compatibility symbols that would only be used when libcrypto is not present, could you please take a look at that changeset and see if it's sensible to adopt the same technique in this changeset? That would avoid the shared library bump and make the change MFC'able.

Indeed, the weak aliases make sense, and avoid the need to bump the major. I'll produce an update patch. Thanks!

In D2216#33, @thomas wrote:

Indeed, the weak aliases make sense, and avoid the need to bump the major. I'll produce an update patch. Thanks!

I believe the patch in D1542 should solve your problem. Please try it.

thomas edited edge metadata.

New version introducing weak aliases

This is to avoid having to rebuild other binaries, or bump the
shared library major version. Succesfully tested against the
original issue as described in PR 199119 (crontab(1) hanging
when using LDAP with SSL for the password database).

This revision now requires review to proceed.Apr 22 2015, 1:39 PM
roberto edited edge metadata.

Ok for me.

This revision is now accepted and ready to land.Apr 22 2015, 1:51 PM
delphij added a reviewer: delphij.

Looks good to me, thanks for working on this!

Unless further discussion takes place, I intend to commit the last version of this change at the end of the week.