[SERVER-49798] Enforce correct library graph for ubsan vptr check builds Created: 22/Jul/20  Updated: 29/Oct/23  Resolved: 12/Oct/20

Status: Closed
Project: Core Server
Component/s: Build
Affects Version/s: None
Fix Version/s: 4.9.0

Type: Improvement Priority: Major - P3
Reporter: Andrew Morrow (Inactive) Assignee: Daniel Moody
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Backports
Related
related to SERVER-51492 write_unit_of_work has circular typei... Closed
related to SERVER-48892 {A,UB}SAN builder should use --link-m... Closed
is related to SERVER-56568 Disable vptr UBSAN checks in dynamic ... Closed
Backwards Compatibility: Fully Compatible
Backport Requested:
v4.4
Sprint: Dev Platform 2020-10-05, Dev Platform 2020-10-19
Participants:

 Description   

The -fsanitize=vptr option, which is enabled by default with -fsanitize=undefined, causes shared libraries to have dependencies on typeinfo symbols which they ordinarily would not.

In an ordinary --link-model=dynamic build, if these symbols were left undefined at the time of linking the library in question, the link would fail because we normally link with -z,defs (or the local equivalent), requiring that each shared library fully express dependencies on all of the libraries required to satisfy all undefined symbols.

However, in a sanitizer build, we disable -z,defs. If we don't, the link fails because the internal symbols the sanitizer injects are undefined.

As a result, it is possible to link a libx.so that has undefined typeinfo references.

Linking all of the programs is not sufficient to find these, because it may be the case that program y happens to also link to a library that provides the missing typeinfo definition that libx.so needs. If all uses of libx.so are always coincidentally accompanied by a library which provides the required symbols, then the entire tree will link.

This however results in a very fragile state, as any perturbation of the link graph structure could suddenly result in libx.so's undefined symbols becoming unsatisfied, but only in the sanitizer build.

The options to address this aren't great:

  • We could disable the vptr checking in ubsan with -fno-sanitize=vptr, but this weakens our sanitizer posture.
  • We could find a way to allow use of -z,defs during our sanitizer builds. This would catch these errors at the time of linking libx.so.
  • We could find a way to cause at least one non-sanitizer builder to similarly emit more dependencies on typeinfo nodes. Such a build would have -z,defs in play and so again the link of libx.so would fail.
  • We could add post-processing to the sanitizer builds to validate each generated shared object had no undefined symbols other than those required by the sanitizer. This is annoying, but it isn't hard.

Here is a list at community/enterprise 86d94765b9030c9aa9adb3899277456e64bbd348/755fc267af1c4475a95e22159b90e0e428d23c28 of the current libraries that have undefined typeinfo references in an undefined behavior sanitizer build.

The shared libraries were produced as follows:

$ python3 ./buildscripts/scons.py --variables-files= --variables-files=etc/scons/mongodbtoolchain_v3_clang.vars --dbg=on --opt=on --allocator=system --sanitize=undefined,address --link-model=dynamic --install-action=hardlink --implicit-cache --build-fast-and-loose=on build/install/lib

The libraries were analyzed as follows (note that libmozjs is excluded because it does not honor our usual -z,defs type rules :

$ ldd -r build/install/lib/*.so 2>&1 | grep 'undefined symbol:' | egrep -v '__asan|__ubsan|__sanitize|__lsan' | c++filt | grep -v libmozjs

We find the following missing typeinfo nodes:

undefined symbol: typeinfo for mongo::ScopedGlobalServiceContextForTest	(/home/andrew/Documents/10gen/dev/src/mongodb/build/install/lib/../lib/libsorted_data_interface_test_harness.so)
undefined symbol: typeinfo for mongo::ScopedGlobalServiceContextForTest	(/home/andrew/Documents/10gen/dev/src/mongodb/build/install/lib/../lib/librecord_store_test_harness.so)
undefined symbol: typeinfo for mongo::ScopedGlobalServiceContextForTest	(build/install/lib/libadditional_wiredtiger_record_store_tests.so)
undefined symbol: typeinfo for mongo::WildcardAccessMethod	(build/install/lib/libcollection_query_info.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libcollection.so)
undefined symbol: typeinfo for mongo::Command	(build/install/lib/libcommand_can_run_here.so)
undefined symbol: typeinfo for mongo::FTSAccessMethod	(build/install/lib/libexpressions_mongod_only.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libflow_control_ticketholder.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libhedging_metrics.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libindex_access_method_factory.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libindex_catalog.so)
undefined symbol: typeinfo for mongo::RecoveryUnit	(build/install/lib/libindex_catalog.so)
undefined symbol: typeinfo for mongo::executor::NetworkInterface	(build/install/lib/libnetwork_interface_thread_pool.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libop_observer.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libplan_yield_policy.so)
undefined symbol: typeinfo for mongo::ScopedGlobalServiceContextForTest	(build/install/lib/librecord_store_test_harness.so)
undefined symbol: typeinfo for mongo::ScopedGlobalServiceContextForTest	(build/install/lib/librecovery_unit_test_harness.so)
undefined symbol: typeinfo for mongo::DBClientBase	(build/install/lib/libreplication_auth.so)
undefined symbol: typeinfo for mongo::SSLConnectionInterface	(build/install/lib/libsocket.so)
undefined symbol: typeinfo for mongo::SSLManagerInterface	(build/install/lib/libsocket.so)
undefined symbol: typeinfo for mongo::ScopedGlobalServiceContextForTest	(build/install/lib/libsorted_data_interface_test_harness.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libstorage_devnull_core.so)
undefined symbol: typeinfo for mongo::OperationContext	(build/install/lib/libwrite_unit_of_work.so)

At least one of these was already known, per SERVER-48741, but the others are novel.

Adding the libraries required to satisfy these may be possible in most cases, but in some cases it may reveal what amount to new cycles in the graph, because satisfying them now introduces a stronger notion of acyclicity than had previously been in force: we now require that the dynamic link graph be acyclic with respect to typeinfo.

Note that this is only true for --link-model=dynamic builds. We could also decide that --link-model=dynamic is inappropriate for undefined sanitizer builds. This would be somewhat unfortunate as we would like to use dynamic linking for the required waterfall builder that uses ubsan, per SERVER-48892.



 Comments   
Comment by Githook User [ 20/Jul/21 ]

Author:

{'name': 'Andrew Morrow', 'email': 'acm@mongodb.com', 'username': 'acmorrow'}

Message: SERVER-56568 Remove LIBDEPS_TYPEINFO in favor of disabling vptr sanitizer for dyanmic builds

This reverts the changes made for SERVER-49798 in favor of disabling
the vptr sanitizer when using ubsan for --link-model=dynamic builds.

(cherry picked from commit 4122e4b7ddc05b49b35aab04ba0d4a70d73f7adf)
Branch: v5.0
https://github.com/mongodb/mongo/commit/8c1a44ead551a98b2cb9e7b93981b7bb2a5df69a

Comment by Githook User [ 09/Jul/21 ]

Author:

{'name': 'Andrew Morrow', 'email': 'acm@mongodb.com', 'username': 'acmorrow'}

Message: SERVER-56568 Remove LIBDEPS_TYPEINFO in favor of disabling vptr sanitizer for dyanmic builds

This reverts the changes made for SERVER-49798 in favor of disabling
the vptr sanitizer when using ubsan for --link-model=dynamic builds.
Branch: master
https://github.com/mongodb/mongo/commit/4122e4b7ddc05b49b35aab04ba0d4a70d73f7adf

Comment by Githook User [ 09/Oct/20 ]

Author:

{'name': 'Daniel Moody', 'email': 'daniel.moody@mongodb.com', 'username': 'dmoody256'}

Message: SERVER-49798 Added LIBDEPS_TYPEINFO for ubsan builds to add typeinfo dependencies.
Branch: master
https://github.com/mongodb/mongo/commit/899679127a04255c0f57222d8f5363b9728382f4

Comment by Githook User [ 09/Oct/20 ]

Author:

{'name': 'Daniel Moody', 'email': 'daniel.moody@mongodb.com', 'username': 'dmoody256'}

Message: SERVER-49798 added LIBDEPS_TYPEINFO for ubsan builds
Branch: master
https://github.com/10gen/mongo-enterprise-modules/commit/c057b59322cd2dbf986892d428177863d64ad44c

Comment by Andrew Morrow (Inactive) [ 29/Jul/20 ]

Here is a first idea at how we could build in tooling to identify such nodes and prevent the introduction of new instances:

diff --git a/SConstruct b/SConstruct
index a30d91a5a3..8f7017383f 100644
--- a/SConstruct
+++ b/SConstruct
@@ -2774,6 +2774,31 @@ def doConfigure(myenv):
         if not myenv.ToolchainIs('clang', 'gcc'):
             env.FatalError('sanitize is only supported with clang or gcc')
 
+        # Some sanitizers, notably the vptr check of ubsan, can cause
+        # additional symbol dependencies to exist. Unfortunately,
+        # building with the sanitizers also requires that we not build
+        # with -z,defs, which means that we cannot make such undefined
+        # symbols errors at link time. We can however hack together
+        # something which looks for undefined typeinfo nodes in the
+        # mongo namespace using `ldd -r`.  See
+        # https://jira.mongodb.org/browse/SERVER-49798 for more
+        # details.
+        if env.TargetOSIs('posix') and not env.TargetOSIs('darwin'):
+            toolmap = { t.upper() : env.WhereIs(t) for t in ['ldd', 'grep', 'c++filt', 'false', 'true'] }
+            if all(toolmap.values()):
+                toolEnv = env.Clone()
+                for k, v in toolmap.items():
+                    toolEnv[k.replace('+', 'X')] = v
+                base_action = env['BUILDERS']['SharedLibrary'].action
+                if not isinstance(base_action, SCons.Action.ListAction):
+                    base_action = SCons.Action.ListAction([base_action])
+                base_action.list.extend([
+                    toolEnv.Action(
+                        "if $LDD -r $$TARGET | $GREP '^undefined symbol: ' | $GREP -v '__[a-z]*san' | $CXXFILT | $GREP 'mongo::' | $GREP -q 'typeinfo for' ; then $FALSE ; else $TRUE ; fi",
+                        "Examining $$TARGET for undefined typeinfo nodes" if not env.Verbose() else None
+                    )
+                ])
+
         if myenv.ToolchainIs('gcc'):
             # GCC's implementation of ASAN depends on libdl.
             env.Append(LIBS=['dl'])

It is pretty janky and could surely be improved (we should probably move that pipeline into an external script), but it would identify affected nodes which would allow us to fix them and then prevent the introduction of others.

Generated at Thu Feb 08 05:20:52 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.