Skip to content

Commit c190aeb

Browse files
yannhamclaude
andcommitted
fix: reject all non-TLSDESC TLS relocations in sanity check
The previous check only forbade GD/LD relocations (DTPMOD/DTPOFF), missing other TLS relocation types like TPOFF64 or GOTTPOFF. Flip from a blocklist to an allowlist: only TLSDESC is accepted. Rename check_no_gd_ld_reloc -> check_tlsdesc_reloc_only accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2d5a183 commit c190aeb

2 files changed

Lines changed: 21 additions & 29 deletions

File tree

libdd-otel-thread-ctx-ffi/tests/elf_properties.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
//! Delegates to [`libdd_otel_thread_ctx::autocheck::check_tls_slot_in`] which
77
//! checks that:
88
//! - `otel_thread_ctx_v1` is exported in the dynamic symbol table as a TLS GLOBAL symbol.
9-
//! - `otel_thread_ctx_v1` does NOT use General Dynamic or Local Dynamic TLS relocations.
9+
//! - `otel_thread_ctx_v1` has no non-TLSDESC TLS relocations.
1010
//!
1111
//! The cdylib path is derived at runtime from the test executable location.
1212
//! Both the test binary and the cdylib live in `target/<[triple/]profile>/deps/`.

libdd-otel-thread-ctx/src/sanity_check.rs

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@
99
//! that the binary was linked with the correct option:
1010
//!
1111
//! - `otel_thread_ctx_v1` is exported as TLS GLOBAL in the dynamic symbol table.
12-
//! - `otel_thread_ctx_v1` is NOT accessed via General Dynamic or Local Dynamic TLS relocations
13-
//! (DTPMOD/DTPOFF) in `.rela.dyn`. The linker may pick TLSDESC or Local Exec depending on
14-
//! optimization; both are acceptable.
12+
//! - `otel_thread_ctx_v1` has no non-TLSDESC TLS relocations in `.rela.dyn`. The linker may pick
13+
//! TLSDESC or Local Exec depending on optimization; both are acceptable. All other TLS relocation
14+
//! types (DTPMOD, DTPOFF, TPOFF, GOTTPOFF, etc.) are rejected.
1515
//!
1616
//! This module is only available on Linux (the only platform that supports the TLSDESC dialect used
1717
//! by this crate) and only when the `sanity-check` feature is enabled.
@@ -29,16 +29,16 @@ pub fn check_tls_slot_in(path: &Path) -> anyhow::Result<()> {
2929
let elf = ElfBytes::<AnyEndian>::minimal_parse(&data)
3030
.with_context(|| format!("failed to parse ELF at {}", path.display()))?;
3131
check_dynsym(&elf)?;
32-
check_no_gd_ld_reloc(&elf)?;
32+
check_tlsdesc_reloc_only(&elf)?;
3333
Ok(())
3434
}
3535

3636
/// Check that the current running module has been linked appropriately to make the OTel shared
3737
/// thread context discoverable.
3838
///
39-
/// Checks that `otel_thread_ctx_v1` is exported as a TLS GLOBAL symbol with no General Dynamic or
40-
/// Local Dynamic TLS relocations. It's an indirect check for TLSDESC, which implies either no
41-
/// relocation (Local Exec/static binary case) or a TLSDESC relocation (dynamic library case).
39+
/// Checks that `otel_thread_ctx_v1` is exported as a TLS GLOBAL symbol and that any TLS
40+
/// relocations targeting it are TLSDESC. No relocation (Local Exec/static binary) is also
41+
/// acceptable.
4242
pub fn sanity_check() -> anyhow::Result<()> {
4343
check_tls_slot_in(&own_elf_path()?)
4444
}
@@ -92,18 +92,14 @@ fn check_dynsym(elf: &ElfBytes<'_, AnyEndian>) -> anyhow::Result<()> {
9292
Ok(())
9393
}
9494

95-
/// Check that there's either no TLS relocation for [SYMBOL] in the given ELF file, or if there is,
96-
/// it's a TLSDESC one. In practice, the check is negative: we check for the absence of relocations
97-
/// associated with the General Dynamic or Local Dynamic TLS access model.
98-
fn check_no_gd_ld_reloc(elf: &ElfBytes<'_, AnyEndian>) -> anyhow::Result<()> {
95+
/// Check that any relocation for [SYMBOL] in `.rela.dyn` is a TLSDESC relocation. No relocation at
96+
/// all (Local Exec / static binary) is also acceptable. All other TLS relocation types (DTPMOD,
97+
/// DTPOFF, TPOFF, GOTTPOFF, etc.) are rejected.
98+
fn check_tlsdesc_reloc_only(elf: &ElfBytes<'_, AnyEndian>) -> anyhow::Result<()> {
9999
#[cfg(target_arch = "x86_64")]
100-
const FORBIDDEN_RELOCS: &[(u32, &str)] =
101-
&[(16, "R_X86_64_DTPMOD64"), (17, "R_X86_64_DTPOFF64")];
100+
const TLSDESC_RELOC: u32 = 36; // R_X86_64_TLSDESC
102101
#[cfg(target_arch = "aarch64")]
103-
const FORBIDDEN_RELOCS: &[(u32, &str)] = &[
104-
(1028, "R_AARCH64_TLS_DTPMOD"),
105-
(1029, "R_AARCH64_TLS_DTPREL"),
106-
];
102+
const TLSDESC_RELOC: u32 = 1031; // R_AARCH64_TLSDESC
107103

108104
let (symtab, strtab) = elf
109105
.dynamic_symbol_table()
@@ -126,22 +122,18 @@ fn check_no_gd_ld_reloc(elf: &ElfBytes<'_, AnyEndian>) -> anyhow::Result<()> {
126122
.context("failed to read section headers")?;
127123

128124
if let Some(rela_shdr) = rela_shdr {
129-
let bad: Vec<&str> = elf
125+
let bad: Vec<u32> = elf
130126
.section_data_as_relas(&rela_shdr)
131127
.context("failed to read .rela.dyn")?
132-
.filter(|r| r.r_sym == sym_idx)
133-
.filter_map(|r| {
134-
FORBIDDEN_RELOCS
135-
.iter()
136-
.find(|(typ, _)| *typ == r.r_type)
137-
.map(|(_, name)| *name)
138-
})
128+
.filter(|r| r.r_sym == sym_idx && r.r_type != TLSDESC_RELOC)
129+
.map(|r| r.r_type)
139130
.collect();
140131
if !bad.is_empty() {
132+
let types: Vec<String> = bad.iter().map(|t| format!("type {t}")).collect();
141133
bail!(
142-
"'{SYMBOL}' has General Dynamic / Local Dynamic relocations in .rela.dyn: {}. \
143-
Expected TLSDESC or Local Exec instead.",
144-
bad.join(", ")
134+
"'{SYMBOL}' has non-TLSDESC relocations in .rela.dyn: {}. \
135+
Only TLSDESC or no relocation (Local Exec) is accepted.",
136+
types.join(", ")
145137
);
146138
}
147139
}

0 commit comments

Comments
 (0)