741258 - ASAN: unresolved symbols in libnssutil3.dylib

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Christoph Diehl [:posidron]]

As discussed in IRC one possible solution would be to add the flag -undefined dynamic_lookup to DSO_LDOPTS in security/coreconf/Darwin.mk

Example:

DSO_LDOPTS = -dynamiclib $(DARWIN_DYLIB_VERSIONS) -install_name @executable_path/$(notdir $@) -headerpad_max_install_names -undefined dynamic_lookup

Comment 1

[Christian Holler (:decoder)]

I reached out to the ASan developers to first ensure that -undefined dynamic_lookup is the right way to do it. Once we are sure about that, I'll take a look where exactly we can add it with best (preferably outside NSS).

Comment 2

[Christian Holler (:decoder)]

According to the ASan developers, we are not supposed to pass -undefined dynamic_lookup ourselves, but instead it's likely that for the linker call here -faddress-sanitizer is not passed (which it should). I'm currently investigating why this flag is dropped at some point in NSS and how to force it.

Comment 3

[Christian Holler (:decoder)]

Attachment: Patch

The proper solution here is to ensure that NSS builds dylibs with -faddress-sanitizer, i.e. that LDFLAGS are correctly passed to NSS dylibs when MOZ_CFLAGS_NSS is set.

I haven't found a good solution to do this without touching NSS itself, but overriding DARWIN_DYLIB_VERSIONS seemed the least intrusive way and it works :)

These changes are only active when building with the --enable-address-sanitizer build option.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 613682 [details] [diff] [review]
Patch

Review of attachment 613682 [details] [diff] [review]:
-----------------------------------------------------------------

Not a mac person.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 613682 [details] [diff] [review]
Patch

Review of attachment 613682 [details] [diff] [review]:
-----------------------------------------------------------------

This feels like the wrong way to fix this. Can you patch NSS instead? I know that's more of a hassle, but I don't like wedging things in where they don't belong.

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 613682 [details] [diff] [review]
Patch

In light of the fact that this code is only hit in a non-default configuration, I'll r+ this. However, I want you to file an NSS bug on adding a way to pass this info down in a more correct way, and mention the bug number in a comment here.

Comment 7

[Christian Holler (:decoder)]

Filed follow-up bug 748423 to discuss and resolve the situation more properly in NSS. I also emailed wtc to ask about the right solution here.

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 613682 [details] [diff] [review]
Patch

http://hg.mozilla.org/integration/mozilla-inbound/rev/c45e97eaba18

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c45e97eaba18