unix: hide symbols beloning to dependencies on Linux

This partially addresses #114.

Previously, we were exporting symbols from dependencies like OpenSSL
and X11 from our dynamic binaries. This can cause problems when other
libraries are loaded into the same address space and attempt to bind
to the symbols being exported by Python.

This commit changes things such that symbols for Python dependencies
are now hidden and can no longer be bound to. Python's own symbols
are still exported of course, as Python extensions (and the `python`
executable) need to call into them.

This change is only implemented on Linux for the moment. Apple
targets require a bit more effort to make work.

As part of this, we update validation to confirm visibility of certain
symbols in ELF binaries. The new validation logic correctly rejects
distributions built before this change.

Author: indygreg

cpython-unix/build-cpython.sh

@@ -682,6 +682,17 @@ fi
 # So we need to set both.
 CFLAGS="${EXTRA_TARGET_CFLAGS} -fPIC -I${TOOLS_PATH}/deps/include -I${TOOLS_PATH}/deps/include/ncursesw"
 LDFLAGS="${EXTRA_TARGET_LDFLAGS} -L${TOOLS_PATH}/deps/lib"
+
+# Some target configurations use `-fvisibility=hidden`. Python's configure handles
+# symbol visibility properly itself. So let it do its thing.
+CFLAGS=${CFLAGS//-fvisibility=hidden/}
+
+# But some symbols from some dependency libraries are still non-hidden for some
+# reason. We force the linker to do our bidding.
+if [ "${PYBUILD_PLATFORM}" != "macos" ]; then
+    LDFLAGS="${LDFLAGS} -Wl,--exclude-libs,ALL"
+fi
+
 EXTRA_CONFIGURE_FLAGS=
 
 if [ "${PYBUILD_PLATFORM}" = "macos" ]; then

cpython-unix/targets.yml

@@ -279,6 +279,7 @@ i686-unknown-linux-gnu:
   target_cc: clang
   target_cflags:
     - '-m32'
+    - '-fvisibility=hidden'
   target_ldflags:
     - '-m32'
   needs:
@@ -606,6 +607,8 @@ x86_64-unknown-linux-gnu:
   host_cc: clang
   host_cxx: clang++
   target_cc: clang
+  target_cflags:
+    - '-fvisibility=hidden'
   needs:
     - bdb
     - binutils
@@ -644,6 +647,7 @@ x86_64_v2-unknown-linux-gnu:
   target_cc: clang
   target_cflags:
     - '-march=x86-64-v2'
+    - '-fvisibility=hidden'
   needs:
     - bdb
     - binutils
@@ -682,6 +686,7 @@ x86_64_v3-unknown-linux-gnu:
   target_cc: clang
   target_cflags:
     - '-march=x86-64-v3'
+    - '-fvisibility=hidden'
   needs:
     - bdb
     - binutils
@@ -720,6 +725,7 @@ x86_64_v4-unknown-linux-gnu:
   target_cc: clang
   target_cflags:
     - '-march=x86-64-v4'
+    - '-fvisibility=hidden'
   needs:
     - bdb
     - binutils
@@ -756,6 +762,8 @@ x86_64-unknown-linux-musl:
   host_cc: clang
   host_cxx: clang++
   target_cc: musl-clang
+  target_cflags:
+    - '-fvisibility=hidden'
   needs:
     - bdb
     - binutils
@@ -795,6 +803,7 @@ x86_64_v2-unknown-linux-musl:
   target_cc: musl-clang
   target_cflags:
     - '-march=x86-64-v2'
+    - '-fvisibility=hidden'
   needs:
     - bdb
     - binutils
@@ -834,6 +843,7 @@ x86_64_v3-unknown-linux-musl:
   target_cc: musl-clang
   target_cflags:
     - '-march=x86-64-v3'
+    - '-fvisibility=hidden'
   needs:
     - bdb
     - binutils
@@ -873,6 +883,7 @@ x86_64_v4-unknown-linux-musl:
   target_cc: musl-clang
   target_cflags:
     - '-march=x86-64-v4'
+    - '-fvisibility=hidden'
   needs:
     - bdb
     - binutils

src/validation.rs

@@ -7,7 +7,10 @@ use {
     anyhow::{anyhow, Context, Result},
     clap::ArgMatches,
     object::{
-        elf::{FileHeader32, FileHeader64},
+        elf::{
+            FileHeader32, FileHeader64, ET_DYN, ET_EXEC, STB_GLOBAL, STB_WEAK, STV_DEFAULT,
+            STV_HIDDEN,
+        },
         macho::{MachHeader32, MachHeader64},
         read::{
             elf::{Dyn, FileHeader, SectionHeader, Sym},
@@ -440,6 +443,75 @@ const MACHO_ALLOWED_WEAK_SYMBOLS_38_NON_AARCH64: &[&str] = &[
     "_statvfs",
 ];
 
+/// Symbols defined in dependency packages.
+///
+/// We use this list to spot test behavior of symbols belonging to dependency packages.
+/// The list is obviously not complete.
+const DEPENDENCY_PACKAGE_SYMBOLS: &[&str] = &[
+    // libX11
+    "XClearWindow",
+    "XFlush",
+    // OpenSSL
+    "BIO_ADDR_new",
+    "BN_new",
+    "DH_new",
+    "SSL_extension_supported",
+    "SSL_read",
+    "CRYPTO_memcmp",
+    "ecp_nistz256_neg",
+    "OPENSSL_instrument_bus",
+    "x25519_fe64_add",
+    // libdb
+    "__txn_begin",
+    // libedit / readline
+    "rl_prompt",
+    "readline",
+    "current_history",
+    "history_expand",
+    // libffi
+    "ffi_call",
+    "ffi_type_void",
+    // ncurses
+    "new_field",
+    "set_field_term",
+    "set_menu_init",
+    "winstr",
+    // gdbm
+    "gdbm_close",
+    "gdbm_import",
+    // sqlite3
+    "sqlite3_initialize",
+    "sqlite3_close",
+    // libxcb
+    "xcb_create_window",
+    "xcb_get_property",
+    // libz
+    "deflateEnd",
+    "gzclose",
+    "inflate",
+    // tix
+    "Tix_DItemCreate",
+    "Tix_GrFormat",
+    // liblzma
+    "lzma_index_init",
+    "lzma_stream_encoder",
+    // tcl
+    "Tcl_Alloc",
+    "Tcl_ChannelName",
+    "Tcl_CreateInterp",
+    // tk
+    "TkBindInit",
+    "TkCreateFrame",
+    "Tk_FreeGC",
+];
+
+const PYTHON_EXPORTED_SYMBOLS: &[&str] = &[
+    "Py_Initialize",
+    "PyList_New",
+    // From limited API.
+    "Py_CompileString",
+];
+
 static WANTED_WINDOWS_STATIC_PATHS: Lazy<BTreeSet<PathBuf>> = Lazy::new(|| {
     [
         PathBuf::from("python/build/lib/libffi.lib"),
@@ -478,6 +550,9 @@ pub struct ValidationContext {
     /// Dynamic libraries required to be loaded.
     pub seen_dylibs: BTreeSet<String>,
 
+    /// Symbols exported from dynamic libpython library.
+    pub libpython_exported_symbols: BTreeSet<String>,
+
     /// Undefined Mach-O symbols that are required / non-weak.
     pub macho_undefined_symbols_strong: RequiredSymbols,
 
@@ -490,6 +565,8 @@ impl ValidationContext {
     pub fn merge(&mut self, other: Self) {
         self.errors.extend(other.errors);
         self.seen_dylibs.extend(other.seen_dylibs);
+        self.libpython_exported_symbols
+            .extend(other.libpython_exported_symbols);
         self.macho_undefined_symbols_strong
             .merge(other.macho_undefined_symbols_strong);
         self.macho_undefined_symbols_weak
@@ -699,6 +776,34 @@ fn validate_elf<'data, Elf: FileHeader<Endian = Endianness>>(
                         }
                     }
                 }
+
+                // Ensure specific symbols in dynamic binaries have proper visibility.
+                if matches!(elf.e_type(endian), ET_EXEC | ET_DYN) {
+                    // Non-local symbols belonging to dependencies should have hidden visibility
+                    // to prevent them from being exported.
+                    if DEPENDENCY_PACKAGE_SYMBOLS.contains(&name.as_ref())
+                        && matches!(symbol.st_bind(), STB_GLOBAL | STB_WEAK)
+                        && symbol.st_visibility() != STV_HIDDEN
+                    {
+                        context.errors.push(format!(
+                            "{} contains non-hidden dependency symbol {}",
+                            path.display(),
+                            name
+                        ));
+                    }
+
+                    if let Some(filename) = path.file_name() {
+                        let filename = filename.to_string_lossy();
+
+                        if filename.starts_with("libpython") && filename.ends_with(".so.1.0") {
+                            if matches!(symbol.st_bind(), STB_GLOBAL | STB_WEAK)
+                                && symbol.st_visibility() == STV_DEFAULT
+                            {
+                                context.libpython_exported_symbols.insert(name.to_string());
+                            }
+                        }
+                    }
+                }
             }
         }
     }
@@ -1121,7 +1226,7 @@ fn validate_distribution(
         }
     }
 
-    // We've now read the contents of the archive. Move on to analyizing the results.
+    // We've now read the contents of the archive. Move on to analyzing the results.
 
     for path in seen_symlink_targets {
         if !seen_paths.contains(&path) {
@@ -1160,6 +1265,21 @@ fn validate_distribution(
         }
     }
 
+    // If we've collected symbols exported from libpython, ensure the Python symbols are
+    // in the set.
+    if !context.libpython_exported_symbols.is_empty() {
+        for symbol in PYTHON_EXPORTED_SYMBOLS {
+            if !context
+                .libpython_exported_symbols
+                .contains(&symbol.to_string())
+            {
+                context
+                    .errors
+                    .push(format!("libpython does not export {}", symbol));
+            }
+        }
+    }
+
     // On Apple Python 3.8 we need to ban most weak symbol references because 3.8 doesn't have
     // the proper runtime guards in place to prevent them from being resolved at runtime,
     // which would lead to a crash. See