* [PATCH v3 1/2] scripts: Add glibcelf.py module
@ 2022-04-11 15:32 Florian Weimer
2022-04-11 15:32 ` [PATCH v3 2/2] Default to --with-default-link=no (bug 25812) Florian Weimer
2022-04-21 15:54 ` [PATCH v3 1/2] scripts: Add glibcelf.py module Siddhesh Poyarekar
0 siblings, 2 replies; 26+ messages in thread
From: Florian Weimer @ 2022-04-11 15:32 UTC (permalink / raw)
To: libc-alpha
Hopefully, this will lead to tests that are easier to maintain. The
current approach of parsing readelf -W output using regular expressions
is not necessarily easier than parsing the ELF data directly.
This module is still somewhat incomplete (e.g., coverage of relocation
types and versioning information is missing), but it is sufficient to
perform basic symbol analysis or program header analysis.
---
v3: Unchanged.
scripts/glibcelf.py | 842 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 842 insertions(+)
create mode 100644 scripts/glibcelf.py
diff --git a/scripts/glibcelf.py b/scripts/glibcelf.py
new file mode 100644
index 0000000000..053b9fa165
--- /dev/null
+++ b/scripts/glibcelf.py
@@ -0,0 +1,842 @@
+#!/usr/bin/python3
+# ELF support functionality for Python.
+# Copyright (C) 2022 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+#
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+"""Basic ELF parser.
+
+Use Image.readfile(path) to read an ELF file into memory and begin
+parsing it.
+
+"""
+
+import collections
+import enum
+import struct
+
+class _OpenIntEnum(enum.IntEnum):
+ """Integer enumeration that supports arbitrary int values."""
+ @classmethod
+ def _missing_(cls, value):
+ # See enum.IntFlag._create_pseudo_member_. This allows
+ # creating of enum constants with arbitrary integer values.
+ pseudo_member = int.__new__(cls, value)
+ pseudo_member._name_ = None
+ pseudo_member._value_ = value
+ return pseudo_member
+
+ def __repr__(self):
+ name = self._name_
+ if name is not None:
+ # The names have prefixes like SHT_, implying their type.
+ return name
+ return '{}({})'.format(self.__class__.__name__, self._value_)
+
+ def __str__(self):
+ name = self._name_
+ if name is not None:
+ return name
+ return str(self._value_)
+
+class ElfClass(_OpenIntEnum):
+ """ELF word size. Type of EI_CLASS values."""
+ ELFCLASSNONE = 0
+ ELFCLASS32 = 1
+ ELFCLASS64 = 2
+
+class ElfData(_OpenIntEnum):
+ """ELF endianess. Type of EI_DATA values."""
+ ELFDATANONE = 0
+ ELFDATA2LSB = 1
+ ELFDATA2MSB = 2
+
+class Machine(_OpenIntEnum):
+ """ELF machine type. Type of values in Ehdr.e_machine field."""
+ EM_NONE = 0
+ EM_M32 = 1
+ EM_SPARC = 2
+ EM_386 = 3
+ EM_68K = 4
+ EM_88K = 5
+ EM_IAMCU = 6
+ EM_860 = 7
+ EM_MIPS = 8
+ EM_S370 = 9
+ EM_MIPS_RS3_LE = 10
+ EM_PARISC = 15
+ EM_VPP500 = 17
+ EM_SPARC32PLUS = 18
+ EM_960 = 19
+ EM_PPC = 20
+ EM_PPC64 = 21
+ EM_S390 = 22
+ EM_SPU = 23
+ EM_V800 = 36
+ EM_FR20 = 37
+ EM_RH32 = 38
+ EM_RCE = 39
+ EM_ARM = 40
+ EM_FAKE_ALPHA = 41
+ EM_SH = 42
+ EM_SPARCV9 = 43
+ EM_TRICORE = 44
+ EM_ARC = 45
+ EM_H8_300 = 46
+ EM_H8_300H = 47
+ EM_H8S = 48
+ EM_H8_500 = 49
+ EM_IA_64 = 50
+ EM_MIPS_X = 51
+ EM_COLDFIRE = 52
+ EM_68HC12 = 53
+ EM_MMA = 54
+ EM_PCP = 55
+ EM_NCPU = 56
+ EM_NDR1 = 57
+ EM_STARCORE = 58
+ EM_ME16 = 59
+ EM_ST100 = 60
+ EM_TINYJ = 61
+ EM_X86_64 = 62
+ EM_PDSP = 63
+ EM_PDP10 = 64
+ EM_PDP11 = 65
+ EM_FX66 = 66
+ EM_ST9PLUS = 67
+ EM_ST7 = 68
+ EM_68HC16 = 69
+ EM_68HC11 = 70
+ EM_68HC08 = 71
+ EM_68HC05 = 72
+ EM_SVX = 73
+ EM_ST19 = 74
+ EM_VAX = 75
+ EM_CRIS = 76
+ EM_JAVELIN = 77
+ EM_FIREPATH = 78
+ EM_ZSP = 79
+ EM_MMIX = 80
+ EM_HUANY = 81
+ EM_PRISM = 82
+ EM_AVR = 83
+ EM_FR30 = 84
+ EM_D10V = 85
+ EM_D30V = 86
+ EM_V850 = 87
+ EM_M32R = 88
+ EM_MN10300 = 89
+ EM_MN10200 = 90
+ EM_PJ = 91
+ EM_OPENRISC = 92
+ EM_ARC_COMPACT = 93
+ EM_XTENSA = 94
+ EM_VIDEOCORE = 95
+ EM_TMM_GPP = 96
+ EM_NS32K = 97
+ EM_TPC = 98
+ EM_SNP1K = 99
+ EM_ST200 = 100
+ EM_IP2K = 101
+ EM_MAX = 102
+ EM_CR = 103
+ EM_F2MC16 = 104
+ EM_MSP430 = 105
+ EM_BLACKFIN = 106
+ EM_SE_C33 = 107
+ EM_SEP = 108
+ EM_ARCA = 109
+ EM_UNICORE = 110
+ EM_EXCESS = 111
+ EM_DXP = 112
+ EM_ALTERA_NIOS2 = 113
+ EM_CRX = 114
+ EM_XGATE = 115
+ EM_C166 = 116
+ EM_M16C = 117
+ EM_DSPIC30F = 118
+ EM_CE = 119
+ EM_M32C = 120
+ EM_TSK3000 = 131
+ EM_RS08 = 132
+ EM_SHARC = 133
+ EM_ECOG2 = 134
+ EM_SCORE7 = 135
+ EM_DSP24 = 136
+ EM_VIDEOCORE3 = 137
+ EM_LATTICEMICO32 = 138
+ EM_SE_C17 = 139
+ EM_TI_C6000 = 140
+ EM_TI_C2000 = 141
+ EM_TI_C5500 = 142
+ EM_TI_ARP32 = 143
+ EM_TI_PRU = 144
+ EM_MMDSP_PLUS = 160
+ EM_CYPRESS_M8C = 161
+ EM_R32C = 162
+ EM_TRIMEDIA = 163
+ EM_QDSP6 = 164
+ EM_8051 = 165
+ EM_STXP7X = 166
+ EM_NDS32 = 167
+ EM_ECOG1X = 168
+ EM_MAXQ30 = 169
+ EM_XIMO16 = 170
+ EM_MANIK = 171
+ EM_CRAYNV2 = 172
+ EM_RX = 173
+ EM_METAG = 174
+ EM_MCST_ELBRUS = 175
+ EM_ECOG16 = 176
+ EM_CR16 = 177
+ EM_ETPU = 178
+ EM_SLE9X = 179
+ EM_L10M = 180
+ EM_K10M = 181
+ EM_AARCH64 = 183
+ EM_AVR32 = 185
+ EM_STM8 = 186
+ EM_TILE64 = 187
+ EM_TILEPRO = 188
+ EM_MICROBLAZE = 189
+ EM_CUDA = 190
+ EM_TILEGX = 191
+ EM_CLOUDSHIELD = 192
+ EM_COREA_1ST = 193
+ EM_COREA_2ND = 194
+ EM_ARCV2 = 195
+ EM_OPEN8 = 196
+ EM_RL78 = 197
+ EM_VIDEOCORE5 = 198
+ EM_78KOR = 199
+ EM_56800EX = 200
+ EM_BA1 = 201
+ EM_BA2 = 202
+ EM_XCORE = 203
+ EM_MCHP_PIC = 204
+ EM_INTELGT = 205
+ EM_KM32 = 210
+ EM_KMX32 = 211
+ EM_EMX16 = 212
+ EM_EMX8 = 213
+ EM_KVARC = 214
+ EM_CDP = 215
+ EM_COGE = 216
+ EM_COOL = 217
+ EM_NORC = 218
+ EM_CSR_KALIMBA = 219
+ EM_Z80 = 220
+ EM_VISIUM = 221
+ EM_FT32 = 222
+ EM_MOXIE = 223
+ EM_AMDGPU = 224
+ EM_RISCV = 243
+ EM_BPF = 247
+ EM_CSKY = 252
+ EM_NUM = 253
+ EM_ALPHA = 0x9026
+
+class Et(_OpenIntEnum):
+ """ELF file type. Type of ET_* values and the Ehdr.e_type field."""
+ ET_NONE = 0
+ ET_REL = 1
+ ET_EXEC = 2
+ ET_DYN = 3
+ ET_CORE = 4
+
+class Shn(_OpenIntEnum):
+ """ELF reserved section indices."""
+ SHN_UNDEF = 0
+ SHN_ABS = 0xfff1
+ SHN_COMMON = 0xfff2
+ SHN_XINDEX = 0xffff
+
+class Sht(_OpenIntEnum):
+ """ELF section types. Type of SHT_* values."""
+ SHT_NULL = 0
+ SHT_PROGBITS = 1
+ SHT_SYMTAB = 2
+ SHT_STRTAB = 3
+ SHT_RELA = 4
+ SHT_HASH = 5
+ SHT_DYNAMIC = 6
+ SHT_NOTE = 7
+ SHT_NOBITS = 8
+ SHT_REL = 9
+ SHT_DYNSYM = 11
+ SHT_INIT_ARRAY = 14
+ SHT_FINI_ARRAY = 15
+ SHT_PREINIT_ARRAY = 16
+ SHT_GROUP = 17
+ SHT_SYMTAB_SHNDX = 18
+ SHT_GNU_ATTRIBUTES = 0x6ffffff5
+ SHT_GNU_HASH = 0x6ffffff6
+ SHT_GNU_LIBLIST = 0x6ffffff7
+ SHT_CHECKSUM = 0x6ffffff8
+ SHT_GNU_verdef = 0x6ffffffd
+ SHT_GNU_verneed = 0x6ffffffe
+ SHT_GNU_versym = 0x6fffffff
+
+class Pf(enum.IntFlag):
+ """Program header flags. Type of Phdr.p_flags values."""
+ PF_X = 1
+ PF_W = 2
+ PF_R = 4
+
+class Shf(enum.IntFlag):
+ """Section flags. Type of Shdr.sh_type values."""
+ SHF_WRITE = 1 << 0
+ SHF_ALLOC = 1 << 1
+ SHF_EXECINSTR = 1 << 2
+ SHF_MERGE = 1 << 4
+ SHF_STRINGS = 1 << 5
+ SHF_INFO_LINK = 1 << 6
+ SHF_LINK_ORDER = 1 << 7
+ SHF_OS_NONCONFORMING = 256
+ SHF_GROUP = 1 << 9
+ SHF_TLS = 1 << 10
+ SHF_COMPRESSED = 1 << 11
+ SHF_GNU_RETAIN = 1 << 21
+ SHF_ORDERED = 1 << 30
+ SHF_RETAIN = 1 << 31
+
+class Stb(_OpenIntEnum):
+ """ELF symbol binding type."""
+ STB_LOCAL = 0
+ STB_GLOBAL = 1
+ STB_WEAK = 3
+ STB_GNU_UNIQUE = 10
+
+class Stt(_OpenIntEnum):
+ """ELF symbol type."""
+ STT_NOTYPE = 0
+ STT_OBJECT = 1
+ STT_FUNC = 2
+ STT_SECTION = 3
+ STT_FILE = 4
+ STT_COMMON = 5
+ STT_TLS = 6
+ STT_GNU_IFUNC = 10
+
+class Pt(_OpenIntEnum):
+ """ELF program header types. Type of Phdr.p_type."""
+ PT_NULL = 0
+ PT_LOAD = 1
+ PT_DYNAMIC = 2
+ PT_INTERP = 3
+ PT_NOTE = 4
+ PT_SHLIB = 5
+ PT_PHDR = 6
+ PT_TLS = 7
+ PT_NUM = 8
+ PT_GNU_EH_FRAME = 0x6474e550
+ PT_GNU_STACK = 0x6474e551
+ PT_GNU_RELRO = 0x6474e552
+ PT_GNU_PROPERTY = 0x6474e553
+ PT_SUNWBSS = 0x6ffffffa
+ PT_SUNWSTACK = 0x6ffffffb
+
+class Dt(_OpenIntEnum):
+ """ELF dynamic segment tags. Type of Dyn.d_val."""
+ DT_NULL = 0
+ DT_NEEDED = 1
+ DT_PLTRELSZ = 2
+ DT_PLTGOT = 3
+ DT_HASH = 4
+ DT_STRTAB = 5
+ DT_SYMTAB = 6
+ DT_RELA = 7
+ DT_RELASZ = 8
+ DT_RELAENT = 9
+ DT_STRSZ = 10
+ DT_SYMENT = 11
+ DT_INIT = 12
+ DT_FINI = 13
+ DT_SONAME = 14
+ DT_RPATH = 15
+ DT_SYMBOLIC = 16
+ DT_REL = 17
+ DT_RELSZ = 18
+ DT_RELENT = 19
+ DT_PLTREL = 20
+ DT_DEBUG = 21
+ DT_TEXTREL = 22
+ DT_JMPREL = 23
+ DT_RUNPATH = 29
+ DT_FLAGS = 30
+ DT_ENCODING = 32
+ DT_PREINIT_ARRAY = 32
+ DT_PREINIT_ARRAYSZ = 33
+ DT_SYMTAB_SHNDX = 34
+ DT_GNU_PRELINKED = 0x6ffffdf5
+ DT_GNU_CONFLICTSZ = 0x6ffffdf6
+ DT_GNU_LIBLISTSZ = 0x6ffffdf7
+ DT_CHECKSUM = 0x6ffffdf8
+ DT_PLTPADSZ = 0x6ffffdf9
+ DT_MOVEENT = 0x6ffffdfa
+ DT_MOVESZ = 0x6ffffdfb
+ DT_FEATURE_1 = 0x6ffffdfc
+ DT_POSFLAG_1 = 0x6ffffdfd
+ DT_SYMINSZ = 0x6ffffdfe
+ DT_SYMINENT = 0x6ffffdff
+ DT_GNU_HASH = 0x6ffffef5
+ DT_TLSDESC_PLT = 0x6ffffef6
+ DT_TLSDESC_GOT = 0x6ffffef7
+ DT_GNU_CONFLICT = 0x6ffffef8
+ DT_GNU_LIBLIST = 0x6ffffef9
+ DT_CONFIG = 0x6ffffefa
+ DT_DEPAUDIT = 0x6ffffefb
+ DT_AUDIT = 0x6ffffefc
+ DT_SYMINFO = 0x6ffffeff
+ DT_VERSYM = 0x6ffffff0
+ DT_RELACOUNT = 0x6ffffff9
+ DT_RELCOUNT = 0x6ffffffa
+ DT_FLAGS_1 = 0x6ffffffb
+ DT_VERDEF = 0x6ffffffc
+ DT_VERDEFNUM = 0x6ffffffd
+ DT_VERNEED = 0x6ffffffe
+ DT_VERNEEDNUM = 0x6fffffff
+ DT_AUXILIARY = 0x7ffffffd
+ DT_FILTER = 0x7fffffff
+
+class StInfo:
+ """ELF symbol binding and type. Type of the Sym.st_info field."""
+ def __init__(self, arg0, arg1=None):
+ if isinstance(arg0, int) and arg1 is None:
+ self.bind = Stb(arg0 >> 4)
+ self.type = Stt(arg0 & 15)
+ else:
+ self.bind = Stb(arg0)
+ self.type = Stt(arg1)
+
+ def value(self):
+ """Returns the raw value for the bind/type combination."""
+ return (self.bind.value() << 4) | (self.type.value())
+
+# Type in an ELF file. Used for deserialization.
+_Layout = collections.namedtuple('_Layout', 'unpack size')
+
+def _define_layouts(baseclass: type, layout32: str, layout64: str,
+ types=None, fields32=None):
+ """Assign variants dict to baseclass.
+
+ The variants dict is indexed by (ElfClass, ElfData) pairs, and its
+ values are _Layout instances.
+
+ """
+ struct32 = struct.Struct(layout32)
+ struct64 = struct.Struct(layout64)
+
+ # Check that the struct formats yield the right number of components.
+ for s in (struct32, struct64):
+ example = s.unpack(b' ' * s.size)
+ if len(example) != len(baseclass._fields):
+ raise ValueError('{!r} yields wrong field count: {} != {}'.format(
+ s.format, len(example), len(baseclass._fields)))
+
+ # Check that field names in types are correct.
+ if types is None:
+ types = ()
+ for n in types:
+ if n not in baseclass._fields:
+ raise ValueError('{} does not have field {!r}'.format(
+ baseclass.__name__, n))
+
+ if fields32 is not None \
+ and set(fields32) != set(baseclass._fields):
+ raise ValueError('{!r} is not a permutation of the fields {!r}'.format(
+ fields32, baseclass._fields))
+
+ def unique_name(name, used_names = (set((baseclass.__name__,))
+ | set(baseclass._fields)
+ | {n.__name__
+ for n in (types or {}).values()})):
+ """Find a name that is not used for a class or field name."""
+ candidate = name
+ n = 0
+ while candidate in used_names:
+ n += 1
+ candidate = '{}{}'.format(name, n)
+ used_names.add(candidate)
+ return candidate
+ blob_name = unique_name('blob')
+ struct_unpack_name = unique_name('struct_unpack')
+ comps_name = unique_name('comps')
+
+ layouts = {}
+ for (bits, elfclass, layout, fields) in (
+ (32, ElfClass.ELFCLASS32, layout32, fields32),
+ (64, ElfClass.ELFCLASS64, layout64, None),
+ ):
+ for (elfdata, structprefix, funcsuffix) in (
+ (ElfData.ELFDATA2LSB, '<', 'LE'),
+ (ElfData.ELFDATA2MSB, '>', 'BE'),
+ ):
+ env = {
+ baseclass.__name__: baseclass,
+ struct_unpack_name: struct.unpack,
+ }
+
+ # Add the type converters.
+ if types:
+ for cls in types.values():
+ env[cls.__name__] = cls
+
+ funcname = ''.join(
+ ('unpack_', baseclass.__name__, str(bits), funcsuffix))
+
+ code = '''
+def {funcname}({blob_name}):
+'''.format(funcname=funcname, blob_name=blob_name)
+
+ indent = ' ' * 4
+ unpack_call = '{}({!r}, {})'.format(
+ struct_unpack_name, structprefix + layout, blob_name)
+ field_names = ', '.join(baseclass._fields)
+ if types is None and fields is None:
+ code += '{}return {}({})\n'.format(
+ indent, baseclass.__name__, unpack_call)
+ else:
+ # Destructuring tuple assignment.
+ if fields is None:
+ code += '{}{} = {}\n'.format(
+ indent, field_names, unpack_call)
+ else:
+ # Use custom field order.
+ code += '{}{} = {}\n'.format(
+ indent, ', '.join(fields), unpack_call)
+
+ # Perform the type conversions.
+ for n in baseclass._fields:
+ if n in types:
+ code += '{}{} = {}({})\n'.format(
+ indent, n, types[n].__name__, n)
+ # Create the named tuple.
+ code += '{}return {}({})\n'.format(
+ indent, baseclass.__name__, field_names)
+
+ exec(code, env)
+ layouts[(elfclass, elfdata)] = _Layout(
+ env[funcname], struct.calcsize(layout))
+ baseclass.layouts = layouts
+
+
+# Corresponds to EI_* indices into Elf*_Ehdr.e_indent.
+class Ident(collections.namedtuple('Ident',
+ 'ei_mag ei_class ei_data ei_version ei_osabi ei_abiversion ei_pad')):
+
+ def __new__(cls, *args):
+ """Construct an object from a blob or its constituent fields."""
+ if len(args) == 1:
+ return cls.unpack(args[0])
+ return cls.__base__.__new__(cls, *args)
+
+ @staticmethod
+ def unpack(blob: memoryview) -> 'Ident':
+ """Parse raws data into a tuple."""
+ ei_mag, ei_class, ei_data, ei_version, ei_osabi, ei_abiversion, \
+ ei_pad = struct.unpack('4s5B7s', blob)
+ return Ident(ei_mag, ElfClass(ei_class), ElfData(ei_data),
+ ei_version, ei_osabi, ei_abiversion, ei_pad)
+ size = 16
+
+# Corresponds to Elf32_Ehdr and Elf64_Ehdr.
+Ehdr = collections.namedtuple('Ehdr',
+ 'e_ident e_type e_machine e_version e_entry e_phoff e_shoff e_flags'
+ + ' e_ehsize e_phentsize e_phnum e_shentsize e_shnum e_shstrndx')
+_define_layouts(Ehdr,
+ layout32='16s2H5I6H',
+ layout64='16s2HI3QI6H',
+ types=dict(e_ident=Ident,
+ e_machine=Machine,
+ e_type=Et,
+ e_shstrndx=Shn))
+
+# Corresponds to Elf32_Phdr and Elf64_Pdhr. Order follows the latter.
+Phdr = collections.namedtuple('Phdr',
+ 'p_type p_flags p_offset p_vaddr p_paddr p_filesz p_memsz p_align')
+_define_layouts(Phdr,
+ layout32='8I',
+ fields32=('p_type', 'p_offset', 'p_vaddr', 'p_paddr',
+ 'p_filesz', 'p_memsz', 'p_flags', 'p_align'),
+ layout64='2I6Q',
+ types=dict(p_type=Pt, p_flags=Pf))
+
+
+# Corresponds to Elf32_Shdr and Elf64_Shdr.
+class Shdr(collections.namedtuple('Shdr',
+ 'sh_name sh_type sh_flags sh_addr sh_offset sh_size sh_link sh_info'
+ + ' sh_addralign sh_entsize')):
+ def resolve(self, strtab: 'StringTable') -> 'Shdr':
+ """Resolve sh_name using a string table."""
+ return self.__class__(strtab.get(self[0]), *self[1:])
+_define_layouts(Shdr,
+ layout32='10I',
+ layout64='2I4Q2I2Q',
+ types=dict(sh_type=Sht,
+ sh_flags=Shf,
+ sh_link=Shn))
+
+# Corresponds to Elf32_Dyn and Elf64_Dyn. The nesting through the
+# d_un union is skipped, and d_ptr is missing (its representation in
+# Python would be identical to d_val).
+Dyn = collections.namedtuple('Dyn', 'd_tag d_val')
+_define_layouts(Dyn,
+ layout32='2i',
+ layout64='2q',
+ types=dict(d_tag=Dt))
+
+# Corresponds to Elf32_Sym and Elf64_Sym.
+class Sym(collections.namedtuple('Sym',
+ 'st_name st_info st_other st_shndx st_value st_size')):
+ def resolve(self, strtab: 'StringTable') -> 'Sym':
+ """Resolve st_name using a string table."""
+ return self.__class__(strtab.get(self[0]), *self[1:])
+_define_layouts(Sym,
+ layout32='3I2BH',
+ layout64='I2BH2Q',
+ fields32=('st_name', 'st_value', 'st_size', 'st_info',
+ 'st_other', 'st_shndx'),
+ types=dict(st_shndx=Shn,
+ st_info=StInfo))
+
+# Corresponds to Elf32_Rel and Elf64_Rel.
+Rel = collections.namedtuple('Rel', 'r_offset r_info')
+_define_layouts(Rel,
+ layout32='2I',
+ layout64='2Q')
+
+# Corresponds to Elf32_Rel and Elf64_Rel.
+Rela = collections.namedtuple('Rela', 'r_offset r_info r_addend')
+_define_layouts(Rela,
+ layout32='3I',
+ layout64='3Q')
+
+class StringTable:
+ """ELF string table."""
+ def __init__(self, blob):
+ """Create a new string table backed by the data in the blob.
+
+ blob: a memoryview-like object
+
+ """
+ self.blob = blob
+
+ def get(self, index) -> bytes:
+ """Returns the null-terminated byte string at the index."""
+ blob = self.blob
+ endindex = index
+ while True:
+ if blob[endindex] == 0:
+ return bytes(blob[index:endindex])
+ endindex += 1
+
+class Image:
+ """ELF image parser."""
+ def __init__(self, image):
+ """Create an ELF image from binary image data.
+
+ image: a memoryview-like object that supports efficient range
+ subscripting.
+
+ """
+ self.image = image
+ ident = self.read(Ident, 0)
+ classdata = (ident.ei_class, ident.ei_data)
+ # Set self.Ehdr etc. to the subtypes with the right parsers.
+ for typ in (Ehdr, Phdr, Shdr, Dyn, Sym, Rel, Rela):
+ setattr(self, typ.__name__, typ.layouts.get(classdata, None))
+
+ if self.Ehdr is not None:
+ self.ehdr = self.read(self.Ehdr, 0)
+ self._shdr_num = self._compute_shdr_num()
+ else:
+ self.ehdr = None
+ self._shdr_num = 0
+
+ self._section = {}
+ self._stringtab = {}
+
+ if self._shdr_num > 0:
+ self._shdr_strtab = self._find_shdr_strtab()
+ else:
+ self._shdr_strtab = None
+
+ @staticmethod
+ def readfile(path: str) -> 'Image':
+ """Reads the ELF file at the specified path."""
+ with open(path, 'rb') as inp:
+ return Image(memoryview(inp.read()))
+
+ def _compute_shdr_num(self) -> int:
+ """Computes the actual number of section headers."""
+ shnum = self.ehdr.e_shnum
+ if shnum == 0:
+ if self.ehdr.e_shoff == 0 or self.ehdr.e_shentsize == 0:
+ # No section headers.
+ return 0
+ # Otherwise the extension mechanism is used (which may be
+ # needed because e_shnum is just 16 bits).
+ return self.read(self.Shdr, self.ehdr.e_shoff).sh_size
+ return shnum
+
+ def _find_shdr_strtab(self) -> StringTable:
+ """Finds the section header string table (maybe via extensions)."""
+ shstrndx = self.ehdr.e_shstrndx
+ if shstrndx == Shn.SHN_XINDEX:
+ shstrndx = self.read(self.Shdr, self.ehdr.e_shoff).sh_link
+ return self._find_stringtab(shstrndx)
+
+ def read(self, typ: type, offset:int ):
+ """Reads an object at a specific offset.
+
+ The type must have been enhanced using _define_variants.
+
+ """
+ return typ.unpack(self.image[offset: offset + typ.size])
+
+ def phdrs(self) -> Phdr:
+ """Generator iterating over the program headers."""
+ if self.ehdr is None:
+ return
+ size = self.ehdr.e_phentsize
+ if size != self.Phdr.size:
+ raise ValueError('Unexpected Phdr size in ELF header: {} != {}'
+ .format(size, self.Phdr.size))
+
+ offset = self.ehdr.e_phoff
+ for _ in range(self.ehdr.e_phnum):
+ yield self.read(self.Phdr, offset)
+ offset += size
+
+ def shdrs(self, resolve: bool=True) -> Shdr:
+ """Generator iterating over the section headers.
+
+ If resolve, section names are automatically translated
+ using the section header string table.
+
+ """
+ if self._shdr_num == 0:
+ return
+
+ size = self.ehdr.e_shentsize
+ if size != self.Shdr.size:
+ raise ValueError('Unexpected Shdr size in ELF header: {} != {}'
+ .format(size, self.Shdr.size))
+
+ offset = self.ehdr.e_shoff
+ for _ in range(self._shdr_num):
+ shdr = self.read(self.Shdr, offset)
+ if resolve:
+ shdr = shdr.resolve(self._shdr_strtab)
+ yield shdr
+ offset += size
+
+ def dynamic(self) -> Dyn:
+ """Generator iterating over the dynamic segment."""
+ for phdr in self.phdrs():
+ if phdr.p_type == Pt.PT_DYNAMIC:
+ # Pick the first dynamic segment, like the loader.
+ if phdr.p_filesz == 0:
+ # Probably separated debuginfo.
+ return
+ offset = phdr.p_offset
+ end = offset + phdr.p_memsz
+ size = self.Dyn.size
+ while True:
+ next_offset = offset + size
+ if next_offset > end:
+ raise ValueError(
+ 'Dynamic segment size {} is not a multiple of Dyn size {}'.format(
+ phdr.p_memsz, size))
+ yield self.read(self.Dyn, offset)
+ if next_offset == end:
+ return
+ offset = next_offset
+
+ def syms(self, shdr: Shdr, resolve: bool=True) -> Sym:
+ """A generator iterating over a symbol table.
+
+ If resolve, symbol names are automatically translated using
+ the string table for the symbol table.
+
+ """
+ assert shdr.sh_type == Sht.SHT_SYMTAB
+ size = shdr.sh_entsize
+ if size != self.Sym.size:
+ raise ValueError('Invalid symbol table entry size {}'.format(size))
+ offset = shdr.sh_offset
+ end = shdr.sh_offset + shdr.sh_size
+ if resolve:
+ strtab = self._find_stringtab(shdr.sh_link)
+ while offset < end:
+ sym = self.read(self.Sym, offset)
+ if resolve:
+ sym = sym.resolve(strtab)
+ yield sym
+ offset += size
+ if offset != end:
+ raise ValueError('Symbol table is not a multiple of entry size')
+
+ def lookup_string(self, strtab_index: int, strtab_offset: int) -> bytes:
+ """Looks up a string in a string table identified by its link index."""
+ try:
+ strtab = self._stringtab[strtab_index]
+ except KeyError:
+ strtab = self._find_stringtab(strtab_index)
+ return strtab.get(strtab_offset)
+
+ def find_section(self, shndx: Shn) -> Shdr:
+ """Returns the section header for the indexed section.
+
+ The section name is not resolved.
+ """
+ try:
+ return self._section[shndx]
+ except KeyError:
+ pass
+ if shndx in Shn:
+ raise ValueError('Reserved section index {}'.format(shndx))
+ idx = shndx.value
+ if idx < 0 or idx > self._shdr_num:
+ raise ValueError('Section index {} out of range [0, {})'.format(
+ idx, self._shdr_num))
+ shdr = self.read(
+ self.Shdr, self.ehdr.e_shoff + idx * self.Shdr.size)
+ self._section[shndx] = shdr
+ return shdr
+
+ def _find_stringtab(self, sh_link: int) -> StringTable:
+ if sh_link in self._stringtab:
+ return self._stringtab
+ if sh_link < 0 or sh_link >= self._shdr_num:
+ raise ValueError('Section index {} out of range [0, {})'.format(
+ sh_link, self._shdr_num))
+ shdr = self.read(
+ self.Shdr, self.ehdr.e_shoff + sh_link * self.Shdr.size)
+ if shdr.sh_type != Sht.SHT_STRTAB:
+ raise ValueError(
+ 'Section {} is not a string table: {}'.format(
+ sh_link, shdr.sh_type))
+ strtab = StringTable(
+ self.image[shdr.sh_offset:shdr.sh_offset + shdr.sh_size])
+ # This could retrain essentially arbitrary amounts of data,
+ # but caching string tables seems important for performance.
+ self._stringtab[sh_link] = strtab
+ return strtab
+
+
+__all__ = [name for name in dir() if name[0].isupper()]
base-commit: 1a85970f41ea1e5abe6da2298a5e8fedcea26b70
--
2.35.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-11 15:32 [PATCH v3 1/2] scripts: Add glibcelf.py module Florian Weimer
@ 2022-04-11 15:32 ` Florian Weimer
2022-04-22 6:12 ` Siddhesh Poyarekar
` (2 more replies)
2022-04-21 15:54 ` [PATCH v3 1/2] scripts: Add glibcelf.py module Siddhesh Poyarekar
1 sibling, 3 replies; 26+ messages in thread
From: Florian Weimer @ 2022-04-11 15:32 UTC (permalink / raw)
To: libc-alpha
This is necessary to place the libio vtables into the RELRO segment.
New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
verify that this is what actually happens.
The new tests fail on ia64 due to lack of (default) RELRO support
inbutils, so they are XFAILed there.
---
v3: Fix INSTALL type and update wording.
INSTALL | 6 ++
configure | 65 +-----------
configure.ac | 55 +----------
elf/Makefile | 33 +++++++
elf/tst-relro-symbols.py | 137 ++++++++++++++++++++++++++
manual/install.texi | 6 ++
sysdeps/unix/sysv/linux/ia64/Makefile | 6 ++
7 files changed, 190 insertions(+), 118 deletions(-)
create mode 100644 elf/tst-relro-symbols.py
diff --git a/INSTALL b/INSTALL
index 63c022d6b9..de150f66eb 100644
--- a/INSTALL
+++ b/INSTALL
@@ -90,6 +90,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
library will still be usable, but functionality may be lost--for
example, you can't build a shared libc with old binutils.
+'--with-default-link=FLAG'
+ With '--with-default-link=yes', the GNU C Library does not use a
+ custom linker scipt for linking its individual shared objects. The
+ default for FLAG is the opposite, 'no', because the custom linker
+ script is needed for full RELRO protection.
+
'--with-nonshared-cflags=CFLAGS'
Use additional compiler flags CFLAGS to build the parts of the
library which are always statically linked into applications and
diff --git a/configure b/configure
index d2f413d05d..650bfd982c 100755
--- a/configure
+++ b/configure
@@ -3375,7 +3375,7 @@ fi
if test "${with_default_link+set}" = set; then :
withval=$with_default_link; use_default_link=$withval
else
- use_default_link=default
+ use_default_link=no
fi
@@ -6184,69 +6184,6 @@ fi
$as_echo "$libc_cv_hashstyle" >&6; }
-# The linker's default -shared behavior is good enough if it
-# does these things that our custom linker scripts ensure that
-# all allocated NOTE sections come first.
-if test "$use_default_link" = default; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sufficient default -shared layout" >&5
-$as_echo_n "checking for sufficient default -shared layout... " >&6; }
-if ${libc_cv_use_default_link+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- libc_cv_use_default_link=no
- cat > conftest.s <<\EOF
- .section .note.a,"a",%note
- .balign 4
- .long 4,4,9
- .string "GNU"
- .string "foo"
- .section .note.b,"a",%note
- .balign 4
- .long 4,4,9
- .string "GNU"
- .string "bar"
-EOF
- if { ac_try=' ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&5'
- { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
- (eval $ac_try) 2>&5
- ac_status=$?
- $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
- test $ac_status = 0; }; } &&
- ac_try=`$READELF -S conftest.so | sed -n \
- '${x;p;}
- s/^ *\[ *[1-9][0-9]*\] *\([^ ][^ ]*\) *\([^ ][^ ]*\) .*$/\2 \1/
- t a
- b
- : a
- H'`
- then
- libc_seen_a=no libc_seen_b=no
- set -- $ac_try
- while test $# -ge 2 -a "$1" = NOTE; do
- case "$2" in
- .note.a) libc_seen_a=yes ;;
- .note.b) libc_seen_b=yes ;;
- esac
- shift 2
- done
- case "$libc_seen_a$libc_seen_b" in
- yesyes)
- libc_cv_use_default_link=yes
- ;;
- *)
- echo >&5 "\
-$libc_seen_a$libc_seen_b from:
-$ac_try"
- ;;
- esac
- fi
- rm -f conftest*
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_use_default_link" >&5
-$as_echo "$libc_cv_use_default_link" >&6; }
- use_default_link=$libc_cv_use_default_link
-fi
-
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for GLOB_DAT reloc" >&5
$as_echo_n "checking for GLOB_DAT reloc... " >&6; }
if ${libc_cv_has_glob_dat+:} false; then :
diff --git a/configure.ac b/configure.ac
index b6a747dece..605efd549d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -153,7 +153,7 @@ AC_ARG_WITH([default-link],
AS_HELP_STRING([--with-default-link],
[do not use explicit linker scripts]),
[use_default_link=$withval],
- [use_default_link=default])
+ [use_default_link=no])
dnl Additional build flags injection.
AC_ARG_WITH([nonshared-cflags],
@@ -1371,59 +1371,6 @@ fi
rm -f conftest*])
AC_SUBST(libc_cv_hashstyle)
-# The linker's default -shared behavior is good enough if it
-# does these things that our custom linker scripts ensure that
-# all allocated NOTE sections come first.
-if test "$use_default_link" = default; then
- AC_CACHE_CHECK([for sufficient default -shared layout],
- libc_cv_use_default_link, [dnl
- libc_cv_use_default_link=no
- cat > conftest.s <<\EOF
- .section .note.a,"a",%note
- .balign 4
- .long 4,4,9
- .string "GNU"
- .string "foo"
- .section .note.b,"a",%note
- .balign 4
- .long 4,4,9
- .string "GNU"
- .string "bar"
-EOF
- if AC_TRY_COMMAND([dnl
- ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&AS_MESSAGE_LOG_FD]) &&
- ac_try=`$READELF -S conftest.so | sed -n \
- ['${x;p;}
- s/^ *\[ *[1-9][0-9]*\] *\([^ ][^ ]*\) *\([^ ][^ ]*\) .*$/\2 \1/
- t a
- b
- : a
- H']`
- then
- libc_seen_a=no libc_seen_b=no
- set -- $ac_try
- while test $# -ge 2 -a "$1" = NOTE; do
- case "$2" in
- .note.a) libc_seen_a=yes ;;
- .note.b) libc_seen_b=yes ;;
- esac
- shift 2
- done
- case "$libc_seen_a$libc_seen_b" in
- yesyes)
- libc_cv_use_default_link=yes
- ;;
- *)
- echo >&AS_MESSAGE_LOG_FD "\
-$libc_seen_a$libc_seen_b from:
-$ac_try"
- ;;
- esac
- fi
- rm -f conftest*])
- use_default_link=$libc_cv_use_default_link
-fi
-
AC_CACHE_CHECK(for GLOB_DAT reloc,
libc_cv_has_glob_dat, [dnl
cat > conftest.c <<EOF
diff --git a/elf/Makefile b/elf/Makefile
index c96924e9c2..366559fc96 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -543,6 +543,39 @@ endif
endif
endif
+tests-special += $(objpfx)tst-relro-ldso.out $(objpfx)tst-relro-libc.out
+$(objpfx)tst-relro-ldso.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
+ $(objpfx)ld.so
+ $(PYTHON) tst-relro-symbols.py $(objpfx)ld.so \
+ --required=_rtld_global_ro \
+ > $@ 2>&1; $(evaluate-test)
+# The optional symbols are present in libc only if the architecture has
+# the GLIBC_2.0 symbol set in libc.
+$(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
+ $(common-objpfx)libc.so
+ $(PYTHON) tst-relro-symbols.py $(common-objpfx)libc.so \
+ --required=_IO_cookie_jumps \
+ --required=_IO_file_jumps \
+ --required=_IO_file_jumps_maybe_mmap \
+ --required=_IO_file_jumps_mmap \
+ --required=_IO_helper_jumps \
+ --required=_IO_mem_jumps \
+ --required=_IO_obstack_jumps \
+ --required=_IO_proc_jumps \
+ --required=_IO_str_chk_jumps \
+ --required=_IO_str_jumps \
+ --required=_IO_strn_jumps \
+ --required=_IO_wfile_jumps \
+ --required=_IO_wfile_jumps_maybe_mmap \
+ --required=_IO_wfile_jumps_mmap \
+ --required=_IO_wmem_jumps \
+ --required=_IO_wstr_jumps \
+ --required=_IO_wstrn_jumps \
+ --optional=_IO_old_cookie_jumps \
+ --optional=_IO_old_file_jumps \
+ --optional=_IO_old_proc_jumps \
+ > $@ 2>&1; $(evaluate-test)
+
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-valgrind-smoke.out
endif
diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
new file mode 100644
index 0000000000..368ea3349f
--- /dev/null
+++ b/elf/tst-relro-symbols.py
@@ -0,0 +1,137 @@
+#!/usr/bin/python3
+# Verify that certain symbols are covered by RELRO.
+# Copyright (C) 2022 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+#
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+"""Analyze a (shared) object to verify that certain symbols are
+present and covered by the PT_GNU_RELRO segment.
+
+"""
+
+import argparse
+import os.path
+import sys
+
+# Make available glibc Python modules.
+sys.path.append(os.path.join(
+ os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
+
+import glibcelf
+
+def find_relro(path: str, img: glibcelf.Image) -> (int, int):
+ """Discover the address range of the PT_GNU_RELRO segment."""
+ for phdr in img.phdrs():
+ if phdr.p_type == glibcelf.Pt.PT_GNU_RELRO:
+ # The computation is not entirely accurate because
+ # _dl_protect_relro in elf/dl-reloc.c rounds both the
+ # start end and downwards using the run-time page size.
+ return phdr.p_vaddr, phdr.p_vaddr + phdr.p_memsz
+ sys.stdout.write('{}: error: no PT_GNU_RELRO segment\n'.format(path))
+ sys.exit(1)
+
+def check_in_relro(kind, relro_begin, relro_end, name, start, size, error):
+ """Check if a section or symbol falls within in the RELRO segment."""
+ end = start + size - 1
+ if not (relro_begin <= start < end < relro_end):
+ error(
+ '{} {!r} of size {} at 0x{:x} is not in RELRO range [0x{:x}, 0x{:x})'.format(
+ kind, name.decode('UTF-8'), start, size,
+ relro_begin, relro_end))
+
+def get_parser():
+ """Return an argument parser for this script."""
+ parser = argparse.ArgumentParser(description=__doc__)
+ parser.add_argument('object', help='path to object file to check')
+ parser.add_argument('--required', metavar='NAME', default=(),
+ help='required symbol names', nargs='*')
+ parser.add_argument('--optional', metavar='NAME', default=(),
+ help='required symbol names', nargs='*')
+ return parser
+
+def main(argv):
+ """The main entry point."""
+ parser = get_parser()
+ opts = parser.parse_args(argv)
+ img = glibcelf.Image.readfile(opts.object)
+
+ required_symbols = frozenset([sym.encode('UTF-8')
+ for sym in opts.required])
+ optional_symbols = frozenset([sym.encode('UTF-8')
+ for sym in opts.optional])
+ check_symbols = required_symbols | optional_symbols
+
+ # Tracks the symbols in check_symbols that have been found.
+ symbols_found = set()
+
+ # Discover the extent of the RELRO segment.
+ relro_begin, relro_end = find_relro(opts.object, img)
+ symbol_table_found = False
+
+ errors = False
+ def error(msg: str) -> None:
+ """Record an error condition and write a message to standard output."""
+ nonlocal errors
+ errors = True
+ sys.stdout.write('{}: error: {}\n'.format(opts.object, msg))
+
+ # Iterate over section headers to find the symbol table.
+ for shdr in img.shdrs():
+ if shdr.sh_type == glibcelf.Sht.SHT_SYMTAB:
+ symbol_table_found = True
+ for sym in img.syms(shdr):
+ if sym.st_name in check_symbols:
+ symbols_found.add(sym.st_name)
+
+ # Validate symbol type, section, and size.
+ if sym.st_info.type != glibcelf.Stt.STT_OBJECT:
+ error('symbol {!r} has wrong type {}'.format(
+ sym.st_name.decode('UTF-8'), sym.st_info.type))
+ if sym.st_shndx in glibcelf.Shn:
+ error('symbol {!r} has reserved section {}'.format(
+ sym.st_name.decode('UTF-8'), sym.st_shndx))
+ continue
+ if sym.st_size == 0:
+ error('symbol {!r} has size zero'.format(
+ sym.st_name.decode('UTF-8')))
+ continue
+
+ check_in_relro('symbol', relro_begin, relro_end,
+ sym.st_name, sym.st_value, sym.st_size,
+ error)
+ continue # SHT_SYMTAB
+ if shdr.sh_name == b'.data.rel.ro' \
+ or shdr.sh_name.startswith(b'.data.rel.ro.'):
+ check_in_relro('section', relro_begin, relro_end,
+ shdr.sh_name, shdr.sh_addr, shdr.sh_size,
+ error)
+ continue
+
+ if required_symbols - symbols_found:
+ for sym in sorted(required_symbols - symbols_found):
+ error('symbol {!r} not found'.format(sym.decode('UTF-8')))
+
+ if errors:
+ sys.exit(1)
+
+ if not symbol_table_found:
+ sys.stdout.write(
+ '{}: warning: no symbol table found (stripped object)\n'.format(
+ opts.object))
+ sys.exit(77)
+
+if __name__ == '__main__':
+ main(sys.argv[1:])
diff --git a/manual/install.texi b/manual/install.texi
index 29c52f2927..fcfb6901e4 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -117,6 +117,12 @@ problem and suppress these constructs, so that the library will still be
usable, but functionality may be lost---for example, you can't build a
shared libc with old binutils.
+@item --with-default-link=@var{FLAG}
+With @code{--with-default-link=yes}, the build system does not use a
+custom linker script for linking shared objects. The default for
+@var{FLAG} is the opposite, @samp{no}, because the custom linker script
+is needed for full RELRO protection.
+
@item --with-nonshared-cflags=@var{cflags}
Use additional compiler flags @var{cflags} to build the parts of the
library which are always statically linked into applications and
diff --git a/sysdeps/unix/sysv/linux/ia64/Makefile b/sysdeps/unix/sysv/linux/ia64/Makefile
index da85ba43e2..c5cc41b367 100644
--- a/sysdeps/unix/sysv/linux/ia64/Makefile
+++ b/sysdeps/unix/sysv/linux/ia64/Makefile
@@ -1,3 +1,9 @@
+ifeq ($(subdir),elf)
+# ia64 does not support PT_GNU_RELRO.
+test-xfail-tst-relro-ldso = yes
+test-xfail-tst-relro-libc = yes
+endif
+
ifeq ($(subdir),misc)
sysdep_headers += sys/rse.h
endif
--
2.35.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/2] scripts: Add glibcelf.py module
2022-04-11 15:32 [PATCH v3 1/2] scripts: Add glibcelf.py module Florian Weimer
2022-04-11 15:32 ` [PATCH v3 2/2] Default to --with-default-link=no (bug 25812) Florian Weimer
@ 2022-04-21 15:54 ` Siddhesh Poyarekar
2022-04-21 20:23 ` Florian Weimer
1 sibling, 1 reply; 26+ messages in thread
From: Siddhesh Poyarekar @ 2022-04-21 15:54 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 11/04/2022 21:02, Florian Weimer via Libc-alpha wrote:
> Hopefully, this will lead to tests that are easier to maintain. The
> current approach of parsing readelf -W output using regular expressions
> is not necessarily easier than parsing the ELF data directly.
>
> This module is still somewhat incomplete (e.g., coverage of relocation
> types and versioning information is missing), but it is sufficient to
> perform basic symbol analysis or program header analysis.
This looks mostly OK, with some comments below. Apart from a couple of
possible typos and nits the comments are light suggestions and not
blockers for commit.
> ---
> v3: Unchanged.
> scripts/glibcelf.py | 842 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 842 insertions(+)
> create mode 100644 scripts/glibcelf.py
>
> diff --git a/scripts/glibcelf.py b/scripts/glibcelf.py
> new file mode 100644
> index 0000000000..053b9fa165
> --- /dev/null
> +++ b/scripts/glibcelf.py
> @@ -0,0 +1,842 @@
> +#!/usr/bin/python3
> +# ELF support functionality for Python.
> +# Copyright (C) 2022 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +#
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
> +
> +"""Basic ELF parser.
> +
> +Use Image.readfile(path) to read an ELF file into memory and begin
> +parsing it.
> +
> +"""
> +
> +import collections
> +import enum
> +import struct
> +
> +class _OpenIntEnum(enum.IntEnum):
> + """Integer enumeration that supports arbitrary int values."""
> + @classmethod
> + def _missing_(cls, value):
> + # See enum.IntFlag._create_pseudo_member_. This allows
> + # creating of enum constants with arbitrary integer values.
> + pseudo_member = int.__new__(cls, value)
> + pseudo_member._name_ = None
> + pseudo_member._value_ = value
> + return pseudo_member
> +
> + def __repr__(self):
> + name = self._name_
> + if name is not None:
> + # The names have prefixes like SHT_, implying their type.
> + return name
> + return '{}({})'.format(self.__class__.__name__, self._value_)
> +
> + def __str__(self):
> + name = self._name_
> + if name is not None:
> + return name
> + return str(self._value_)
> +
> +class ElfClass(_OpenIntEnum):
> + """ELF word size. Type of EI_CLASS values."""
> + ELFCLASSNONE = 0
> + ELFCLASS32 = 1
> + ELFCLASS64 = 2
> +
> +class ElfData(_OpenIntEnum):
> + """ELF endianess. Type of EI_DATA values."""
> + ELFDATANONE = 0
> + ELFDATA2LSB = 1
> + ELFDATA2MSB = 2
> +
> +class Machine(_OpenIntEnum):
> + """ELF machine type. Type of values in Ehdr.e_machine field."""
> + EM_NONE = 0
> + EM_M32 = 1
> + EM_SPARC = 2
> + EM_386 = 3
> + EM_68K = 4
> + EM_88K = 5
> + EM_IAMCU = 6
> + EM_860 = 7
> + EM_MIPS = 8
> + EM_S370 = 9
> + EM_MIPS_RS3_LE = 10
> + EM_PARISC = 15
> + EM_VPP500 = 17
> + EM_SPARC32PLUS = 18
> + EM_960 = 19
> + EM_PPC = 20
> + EM_PPC64 = 21
> + EM_S390 = 22
> + EM_SPU = 23
> + EM_V800 = 36
> + EM_FR20 = 37
> + EM_RH32 = 38
> + EM_RCE = 39
> + EM_ARM = 40
> + EM_FAKE_ALPHA = 41
> + EM_SH = 42
> + EM_SPARCV9 = 43
> + EM_TRICORE = 44
> + EM_ARC = 45
> + EM_H8_300 = 46
> + EM_H8_300H = 47
> + EM_H8S = 48
> + EM_H8_500 = 49
> + EM_IA_64 = 50
> + EM_MIPS_X = 51
> + EM_COLDFIRE = 52
> + EM_68HC12 = 53
> + EM_MMA = 54
> + EM_PCP = 55
> + EM_NCPU = 56
> + EM_NDR1 = 57
> + EM_STARCORE = 58
> + EM_ME16 = 59
> + EM_ST100 = 60
> + EM_TINYJ = 61
> + EM_X86_64 = 62
> + EM_PDSP = 63
> + EM_PDP10 = 64
> + EM_PDP11 = 65
> + EM_FX66 = 66
> + EM_ST9PLUS = 67
> + EM_ST7 = 68
> + EM_68HC16 = 69
> + EM_68HC11 = 70
> + EM_68HC08 = 71
> + EM_68HC05 = 72
> + EM_SVX = 73
> + EM_ST19 = 74
> + EM_VAX = 75
> + EM_CRIS = 76
> + EM_JAVELIN = 77
> + EM_FIREPATH = 78
> + EM_ZSP = 79
> + EM_MMIX = 80
> + EM_HUANY = 81
> + EM_PRISM = 82
> + EM_AVR = 83
> + EM_FR30 = 84
> + EM_D10V = 85
> + EM_D30V = 86
> + EM_V850 = 87
> + EM_M32R = 88
> + EM_MN10300 = 89
> + EM_MN10200 = 90
> + EM_PJ = 91
> + EM_OPENRISC = 92
> + EM_ARC_COMPACT = 93
> + EM_XTENSA = 94
> + EM_VIDEOCORE = 95
> + EM_TMM_GPP = 96
> + EM_NS32K = 97
> + EM_TPC = 98
> + EM_SNP1K = 99
> + EM_ST200 = 100
> + EM_IP2K = 101
> + EM_MAX = 102
> + EM_CR = 103
> + EM_F2MC16 = 104
> + EM_MSP430 = 105
> + EM_BLACKFIN = 106
> + EM_SE_C33 = 107
> + EM_SEP = 108
> + EM_ARCA = 109
> + EM_UNICORE = 110
> + EM_EXCESS = 111
> + EM_DXP = 112
> + EM_ALTERA_NIOS2 = 113
> + EM_CRX = 114
> + EM_XGATE = 115
> + EM_C166 = 116
> + EM_M16C = 117
> + EM_DSPIC30F = 118
> + EM_CE = 119
> + EM_M32C = 120
> + EM_TSK3000 = 131
> + EM_RS08 = 132
> + EM_SHARC = 133
> + EM_ECOG2 = 134
> + EM_SCORE7 = 135
> + EM_DSP24 = 136
> + EM_VIDEOCORE3 = 137
> + EM_LATTICEMICO32 = 138
> + EM_SE_C17 = 139
> + EM_TI_C6000 = 140
> + EM_TI_C2000 = 141
> + EM_TI_C5500 = 142
> + EM_TI_ARP32 = 143
> + EM_TI_PRU = 144
> + EM_MMDSP_PLUS = 160
> + EM_CYPRESS_M8C = 161
> + EM_R32C = 162
> + EM_TRIMEDIA = 163
> + EM_QDSP6 = 164
> + EM_8051 = 165
> + EM_STXP7X = 166
> + EM_NDS32 = 167
> + EM_ECOG1X = 168
> + EM_MAXQ30 = 169
> + EM_XIMO16 = 170
> + EM_MANIK = 171
> + EM_CRAYNV2 = 172
> + EM_RX = 173
> + EM_METAG = 174
> + EM_MCST_ELBRUS = 175
> + EM_ECOG16 = 176
> + EM_CR16 = 177
> + EM_ETPU = 178
> + EM_SLE9X = 179
> + EM_L10M = 180
> + EM_K10M = 181
> + EM_AARCH64 = 183
> + EM_AVR32 = 185
> + EM_STM8 = 186
> + EM_TILE64 = 187
> + EM_TILEPRO = 188
> + EM_MICROBLAZE = 189
> + EM_CUDA = 190
> + EM_TILEGX = 191
> + EM_CLOUDSHIELD = 192
> + EM_COREA_1ST = 193
> + EM_COREA_2ND = 194
> + EM_ARCV2 = 195
> + EM_OPEN8 = 196
> + EM_RL78 = 197
> + EM_VIDEOCORE5 = 198
> + EM_78KOR = 199
> + EM_56800EX = 200
> + EM_BA1 = 201
> + EM_BA2 = 202
> + EM_XCORE = 203
> + EM_MCHP_PIC = 204
> + EM_INTELGT = 205
> + EM_KM32 = 210
> + EM_KMX32 = 211
> + EM_EMX16 = 212
> + EM_EMX8 = 213
> + EM_KVARC = 214
> + EM_CDP = 215
> + EM_COGE = 216
> + EM_COOL = 217
> + EM_NORC = 218
> + EM_CSR_KALIMBA = 219
> + EM_Z80 = 220
> + EM_VISIUM = 221
> + EM_FT32 = 222
> + EM_MOXIE = 223
> + EM_AMDGPU = 224
> + EM_RISCV = 243
> + EM_BPF = 247
> + EM_CSKY = 252
> + EM_NUM = 253
> + EM_ALPHA = 0x9026
> +
> +class Et(_OpenIntEnum):
> + """ELF file type. Type of ET_* values and the Ehdr.e_type field."""
> + ET_NONE = 0
> + ET_REL = 1
> + ET_EXEC = 2
> + ET_DYN = 3
> + ET_CORE = 4
> +
> +class Shn(_OpenIntEnum):
> + """ELF reserved section indices."""
> + SHN_UNDEF = 0
> + SHN_ABS = 0xfff1
> + SHN_COMMON = 0xfff2
> + SHN_XINDEX = 0xffff
> +
> +class Sht(_OpenIntEnum):
> + """ELF section types. Type of SHT_* values."""
> + SHT_NULL = 0
> + SHT_PROGBITS = 1
> + SHT_SYMTAB = 2
> + SHT_STRTAB = 3
> + SHT_RELA = 4
> + SHT_HASH = 5
> + SHT_DYNAMIC = 6
> + SHT_NOTE = 7
> + SHT_NOBITS = 8
> + SHT_REL = 9
> + SHT_DYNSYM = 11
> + SHT_INIT_ARRAY = 14
> + SHT_FINI_ARRAY = 15
> + SHT_PREINIT_ARRAY = 16
> + SHT_GROUP = 17
> + SHT_SYMTAB_SHNDX = 18
> + SHT_GNU_ATTRIBUTES = 0x6ffffff5
> + SHT_GNU_HASH = 0x6ffffff6
> + SHT_GNU_LIBLIST = 0x6ffffff7
> + SHT_CHECKSUM = 0x6ffffff8
> + SHT_GNU_verdef = 0x6ffffffd
> + SHT_GNU_verneed = 0x6ffffffe
> + SHT_GNU_versym = 0x6fffffff
> +
> +class Pf(enum.IntFlag):
> + """Program header flags. Type of Phdr.p_flags values."""
> + PF_X = 1
> + PF_W = 2
> + PF_R = 4
> +
> +class Shf(enum.IntFlag):
> + """Section flags. Type of Shdr.sh_type values."""
> + SHF_WRITE = 1 << 0
> + SHF_ALLOC = 1 << 1
> + SHF_EXECINSTR = 1 << 2
> + SHF_MERGE = 1 << 4
> + SHF_STRINGS = 1 << 5
> + SHF_INFO_LINK = 1 << 6
> + SHF_LINK_ORDER = 1 << 7
> + SHF_OS_NONCONFORMING = 256
> + SHF_GROUP = 1 << 9
> + SHF_TLS = 1 << 10
> + SHF_COMPRESSED = 1 << 11
> + SHF_GNU_RETAIN = 1 << 21
> + SHF_ORDERED = 1 << 30
> + SHF_RETAIN = 1 << 31
> +
> +class Stb(_OpenIntEnum):
> + """ELF symbol binding type."""
> + STB_LOCAL = 0
> + STB_GLOBAL = 1
> + STB_WEAK = 3
> + STB_GNU_UNIQUE = 10
> +
> +class Stt(_OpenIntEnum):
> + """ELF symbol type."""
> + STT_NOTYPE = 0
> + STT_OBJECT = 1
> + STT_FUNC = 2
> + STT_SECTION = 3
> + STT_FILE = 4
> + STT_COMMON = 5
> + STT_TLS = 6
> + STT_GNU_IFUNC = 10
> +
> +class Pt(_OpenIntEnum):
> + """ELF program header types. Type of Phdr.p_type."""
> + PT_NULL = 0
> + PT_LOAD = 1
> + PT_DYNAMIC = 2
> + PT_INTERP = 3
> + PT_NOTE = 4
> + PT_SHLIB = 5
> + PT_PHDR = 6
> + PT_TLS = 7
> + PT_NUM = 8
> + PT_GNU_EH_FRAME = 0x6474e550
> + PT_GNU_STACK = 0x6474e551
> + PT_GNU_RELRO = 0x6474e552
> + PT_GNU_PROPERTY = 0x6474e553
> + PT_SUNWBSS = 0x6ffffffa
> + PT_SUNWSTACK = 0x6ffffffb
> +
> +class Dt(_OpenIntEnum):
> + """ELF dynamic segment tags. Type of Dyn.d_val."""
> + DT_NULL = 0
> + DT_NEEDED = 1
> + DT_PLTRELSZ = 2
> + DT_PLTGOT = 3
> + DT_HASH = 4
> + DT_STRTAB = 5
> + DT_SYMTAB = 6
> + DT_RELA = 7
> + DT_RELASZ = 8
> + DT_RELAENT = 9
> + DT_STRSZ = 10
> + DT_SYMENT = 11
> + DT_INIT = 12
> + DT_FINI = 13
> + DT_SONAME = 14
> + DT_RPATH = 15
> + DT_SYMBOLIC = 16
> + DT_REL = 17
> + DT_RELSZ = 18
> + DT_RELENT = 19
> + DT_PLTREL = 20
> + DT_DEBUG = 21
> + DT_TEXTREL = 22
> + DT_JMPREL = 23
> + DT_RUNPATH = 29
> + DT_FLAGS = 30
> + DT_ENCODING = 32
> + DT_PREINIT_ARRAY = 32
> + DT_PREINIT_ARRAYSZ = 33
> + DT_SYMTAB_SHNDX = 34
> + DT_GNU_PRELINKED = 0x6ffffdf5
> + DT_GNU_CONFLICTSZ = 0x6ffffdf6
> + DT_GNU_LIBLISTSZ = 0x6ffffdf7
> + DT_CHECKSUM = 0x6ffffdf8
> + DT_PLTPADSZ = 0x6ffffdf9
> + DT_MOVEENT = 0x6ffffdfa
> + DT_MOVESZ = 0x6ffffdfb
> + DT_FEATURE_1 = 0x6ffffdfc
> + DT_POSFLAG_1 = 0x6ffffdfd
> + DT_SYMINSZ = 0x6ffffdfe
> + DT_SYMINENT = 0x6ffffdff
> + DT_GNU_HASH = 0x6ffffef5
> + DT_TLSDESC_PLT = 0x6ffffef6
> + DT_TLSDESC_GOT = 0x6ffffef7
> + DT_GNU_CONFLICT = 0x6ffffef8
> + DT_GNU_LIBLIST = 0x6ffffef9
> + DT_CONFIG = 0x6ffffefa
> + DT_DEPAUDIT = 0x6ffffefb
> + DT_AUDIT = 0x6ffffefc
> + DT_SYMINFO = 0x6ffffeff
> + DT_VERSYM = 0x6ffffff0
> + DT_RELACOUNT = 0x6ffffff9
> + DT_RELCOUNT = 0x6ffffffa
> + DT_FLAGS_1 = 0x6ffffffb
> + DT_VERDEF = 0x6ffffffc
> + DT_VERDEFNUM = 0x6ffffffd
> + DT_VERNEED = 0x6ffffffe
> + DT_VERNEEDNUM = 0x6fffffff
> + DT_AUXILIARY = 0x7ffffffd
> + DT_FILTER = 0x7fffffff
Could we generate all these from elf.h, or generate both, this and elf.h
from some common source? At the moment there are a lot of
platform-specific macros missing and some common ones too, e.g.
DT_BIND_NOW. Further, STB_WEAK appears to have a different value from
that in elf.h and there's an SHF_RETAIN that doesn't have a
corresponding macro in elf.h.
We could avoid all this if it is all generated from a single source of
truth. Minimally, ISTM that STB_WEAK needs to be fixed and SHF_RETAIN
dropped, but the rest is more a suggestion to ease future maintenance so
you could either do it now or later as the script evolves.
> +
> +class StInfo:
> + """ELF symbol binding and type. Type of the Sym.st_info field."""
> + def __init__(self, arg0, arg1=None):
> + if isinstance(arg0, int) and arg1 is None:
> + self.bind = Stb(arg0 >> 4)
> + self.type = Stt(arg0 & 15)
> + else:
> + self.bind = Stb(arg0)
> + self.type = Stt(arg1)
> +
> + def value(self):
> + """Returns the raw value for the bind/type combination."""
> + return (self.bind.value() << 4) | (self.type.value())
OK.
> +
> +# Type in an ELF file. Used for deserialization.
> +_Layout = collections.namedtuple('_Layout', 'unpack size')
> +
> +def _define_layouts(baseclass: type, layout32: str, layout64: str,
> + types=None, fields32=None):
> + """Assign variants dict to baseclass.
> +
> + The variants dict is indexed by (ElfClass, ElfData) pairs, and its
> + values are _Layout instances.
> +
> + """
> + struct32 = struct.Struct(layout32)
> + struct64 = struct.Struct(layout64)
> +
> + # Check that the struct formats yield the right number of components.
> + for s in (struct32, struct64):
> + example = s.unpack(b' ' * s.size)
> + if len(example) != len(baseclass._fields):
> + raise ValueError('{!r} yields wrong field count: {} != {}'.format(
> + s.format, len(example), len(baseclass._fields)))
> +
> + # Check that field names in types are correct.
> + if types is None:
> + types = ()
> + for n in types:
> + if n not in baseclass._fields:
> + raise ValueError('{} does not have field {!r}'.format(
> + baseclass.__name__, n))
> +
> + if fields32 is not None \
> + and set(fields32) != set(baseclass._fields):
> + raise ValueError('{!r} is not a permutation of the fields {!r}'.format(
> + fields32, baseclass._fields))
Validations. OK.
> +
> + def unique_name(name, used_names = (set((baseclass.__name__,))
> + | set(baseclass._fields)
> + | {n.__name__
> + for n in (types or {}).values()})):
> + """Find a name that is not used for a class or field name."""
> + candidate = name
> + n = 0
> + while candidate in used_names:
> + n += 1
> + candidate = '{}{}'.format(name, n)
> + used_names.add(candidate)
> + return candidate
Another newline here please, so that the nested function stands out a bit.
> + blob_name = unique_name('blob')
> + struct_unpack_name = unique_name('struct_unpack')
> + comps_name = unique_name('comps')
> +
> + layouts = {}
> + for (bits, elfclass, layout, fields) in (
> + (32, ElfClass.ELFCLASS32, layout32, fields32),
> + (64, ElfClass.ELFCLASS64, layout64, None),
> + ):
> + for (elfdata, structprefix, funcsuffix) in (
> + (ElfData.ELFDATA2LSB, '<', 'LE'),
> + (ElfData.ELFDATA2MSB, '>', 'BE'),
> + ):
> + env = {
> + baseclass.__name__: baseclass,
> + struct_unpack_name: struct.unpack,
> + }
> +
> + # Add the type converters.
> + if types:
> + for cls in types.values():
> + env[cls.__name__] = cls
> +
> + funcname = ''.join(
> + ('unpack_', baseclass.__name__, str(bits), funcsuffix))
> +
> + code = '''
> +def {funcname}({blob_name}):
> +'''.format(funcname=funcname, blob_name=blob_name)
> +
> + indent = ' ' * 4
> + unpack_call = '{}({!r}, {})'.format(
> + struct_unpack_name, structprefix + layout, blob_name)
> + field_names = ', '.join(baseclass._fields)
> + if types is None and fields is None:
> + code += '{}return {}({})\n'.format(
> + indent, baseclass.__name__, unpack_call)
> + else:
> + # Destructuring tuple assignment.
> + if fields is None:
> + code += '{}{} = {}\n'.format(
> + indent, field_names, unpack_call)
> + else:
> + # Use custom field order.
> + code += '{}{} = {}\n'.format(
> + indent, ', '.join(fields), unpack_call)
> +
> + # Perform the type conversions.
> + for n in baseclass._fields:
> + if n in types:
> + code += '{}{} = {}({})\n'.format(
> + indent, n, types[n].__name__, n)
> + # Create the named tuple.
> + code += '{}return {}({})\n'.format(
> + indent, baseclass.__name__, field_names)
> +
> + exec(code, env)
> + layouts[(elfclass, elfdata)] = _Layout(
> + env[funcname], struct.calcsize(layout))
Building layouts for all wordsize and endianness permutations. OK.
> + baseclass.layouts = layouts
> +
> +
> +# Corresponds to EI_* indices into Elf*_Ehdr.e_indent.
> +class Ident(collections.namedtuple('Ident',
> + 'ei_mag ei_class ei_data ei_version ei_osabi ei_abiversion ei_pad')):
> +
> + def __new__(cls, *args):
> + """Construct an object from a blob or its constituent fields."""
> + if len(args) == 1:
> + return cls.unpack(args[0])
> + return cls.__base__.__new__(cls, *args)
> +
> + @staticmethod
> + def unpack(blob: memoryview) -> 'Ident':
> + """Parse raws data into a tuple."""
> + ei_mag, ei_class, ei_data, ei_version, ei_osabi, ei_abiversion, \
> + ei_pad = struct.unpack('4s5B7s', blob)
> + return Ident(ei_mag, ElfClass(ei_class), ElfData(ei_data),
> + ei_version, ei_osabi, ei_abiversion, ei_pad)
> + size = 16
OK.
> +
> +# Corresponds to Elf32_Ehdr and Elf64_Ehdr.
> +Ehdr = collections.namedtuple('Ehdr',
> + 'e_ident e_type e_machine e_version e_entry e_phoff e_shoff e_flags'
> + + ' e_ehsize e_phentsize e_phnum e_shentsize e_shnum e_shstrndx')
> +_define_layouts(Ehdr,
> + layout32='16s2H5I6H',
> + layout64='16s2HI3QI6H',
Verified against the structs. OK.
> + types=dict(e_ident=Ident,
> + e_machine=Machine,
> + e_type=Et,
> + e_shstrndx=Shn))
> +
> +# Corresponds to Elf32_Phdr and Elf64_Pdhr. Order follows the latter.
> +Phdr = collections.namedtuple('Phdr',
> + 'p_type p_flags p_offset p_vaddr p_paddr p_filesz p_memsz p_align')
> +_define_layouts(Phdr,
> + layout32='8I',
> + fields32=('p_type', 'p_offset', 'p_vaddr', 'p_paddr',
> + 'p_filesz', 'p_memsz', 'p_flags', 'p_align'),
> + layout64='2I6Q',
> + types=dict(p_type=Pt, p_flags=Pf))
Likewise. OK.
> +
> +
> +# Corresponds to Elf32_Shdr and Elf64_Shdr.
> +class Shdr(collections.namedtuple('Shdr',
> + 'sh_name sh_type sh_flags sh_addr sh_offset sh_size sh_link sh_info'
> + + ' sh_addralign sh_entsize')):
> + def resolve(self, strtab: 'StringTable') -> 'Shdr':
> + """Resolve sh_name using a string table."""
> + return self.__class__(strtab.get(self[0]), *self[1:])
> +_define_layouts(Shdr,
> + layout32='10I',
> + layout64='2I4Q2I2Q',
> + types=dict(sh_type=Sht,
> + sh_flags=Shf,
> + sh_link=Shn))
OK.
> +
> +# Corresponds to Elf32_Dyn and Elf64_Dyn. The nesting through the
> +# d_un union is skipped, and d_ptr is missing (its representation in
> +# Python would be identical to d_val).
> +Dyn = collections.namedtuple('Dyn', 'd_tag d_val')
> +_define_layouts(Dyn,
> + layout32='2i',
> + layout64='2q',
> + types=dict(d_tag=Dt))
OK.
> +
> +# Corresponds to Elf32_Sym and Elf64_Sym.
> +class Sym(collections.namedtuple('Sym',
> + 'st_name st_info st_other st_shndx st_value st_size')):
> + def resolve(self, strtab: 'StringTable') -> 'Sym':
> + """Resolve st_name using a string table."""
> + return self.__class__(strtab.get(self[0]), *self[1:])
> +_define_layouts(Sym,
> + layout32='3I2BH',
> + layout64='I2BH2Q',
> + fields32=('st_name', 'st_value', 'st_size', 'st_info',
> + 'st_other', 'st_shndx'),
> + types=dict(st_shndx=Shn,
> + st_info=StInfo))
OK.
> +
> +# Corresponds to Elf32_Rel and Elf64_Rel.
> +Rel = collections.namedtuple('Rel', 'r_offset r_info')
> +_define_layouts(Rel,
> + layout32='2I',
> + layout64='2Q')
> +
OK.
> +# Corresponds to Elf32_Rel and Elf64_Rel.
> +Rela = collections.namedtuple('Rela', 'r_offset r_info r_addend')
> +_define_layouts(Rela,
> + layout32='3I',
> + layout64='3Q')
OK.
> +
> +class StringTable:
> + """ELF string table."""
> + def __init__(self, blob):
> + """Create a new string table backed by the data in the blob.
> +
> + blob: a memoryview-like object
> +
> + """
> + self.blob = blob
> +
> + def get(self, index) -> bytes:
> + """Returns the null-terminated byte string at the index."""
> + blob = self.blob
> + endindex = index
> + while True:
> + if blob[endindex] == 0:
> + return bytes(blob[index:endindex])
> + endindex += 1
OK.
> +
> +class Image:
> + """ELF image parser."""
> + def __init__(self, image):
> + """Create an ELF image from binary image data.
> +
> + image: a memoryview-like object that supports efficient range
> + subscripting.
> +
> + """
> + self.image = image
> + ident = self.read(Ident, 0)
> + classdata = (ident.ei_class, ident.ei_data)
> + # Set self.Ehdr etc. to the subtypes with the right parsers.
> + for typ in (Ehdr, Phdr, Shdr, Dyn, Sym, Rel, Rela):
> + setattr(self, typ.__name__, typ.layouts.get(classdata, None))
> +
> + if self.Ehdr is not None:
> + self.ehdr = self.read(self.Ehdr, 0)
> + self._shdr_num = self._compute_shdr_num()
> + else:
> + self.ehdr = None
> + self._shdr_num = 0
> +
> + self._section = {}
> + self._stringtab = {}
> +
> + if self._shdr_num > 0:
> + self._shdr_strtab = self._find_shdr_strtab()
> + else:
> + self._shdr_strtab = None
OK.
> +
> + @staticmethod
> + def readfile(path: str) -> 'Image':
> + """Reads the ELF file at the specified path."""
> + with open(path, 'rb') as inp:
> + return Image(memoryview(inp.read()))
OK.
> +
> + def _compute_shdr_num(self) -> int:
> + """Computes the actual number of section headers."""
> + shnum = self.ehdr.e_shnum
> + if shnum == 0:
> + if self.ehdr.e_shoff == 0 or self.ehdr.e_shentsize == 0:
> + # No section headers.
> + return 0
> + # Otherwise the extension mechanism is used (which may be
> + # needed because e_shnum is just 16 bits).
> + return self.read(self.Shdr, self.ehdr.e_shoff).sh_size
> + return shnum
OK.
> +
> + def _find_shdr_strtab(self) -> StringTable:
> + """Finds the section header string table (maybe via extensions)."""
> + shstrndx = self.ehdr.e_shstrndx
> + if shstrndx == Shn.SHN_XINDEX:
> + shstrndx = self.read(self.Shdr, self.ehdr.e_shoff).sh_link
> + return self._find_stringtab(shstrndx)
OK.
> +
> + def read(self, typ: type, offset:int ):
> + """Reads an object at a specific offset.
> +
> + The type must have been enhanced using _define_variants.
> +
> + """
> + return typ.unpack(self.image[offset: offset + typ.size])
OK.
> +
> + def phdrs(self) -> Phdr:
> + """Generator iterating over the program headers."""
> + if self.ehdr is None:
> + return
> + size = self.ehdr.e_phentsize
> + if size != self.Phdr.size:
> + raise ValueError('Unexpected Phdr size in ELF header: {} != {}'
> + .format(size, self.Phdr.size))
> +
> + offset = self.ehdr.e_phoff
> + for _ in range(self.ehdr.e_phnum):
> + yield self.read(self.Phdr, offset)
> + offset += size
OK.
> +
> + def shdrs(self, resolve: bool=True) -> Shdr:
> + """Generator iterating over the section headers.
> +
> + If resolve, section names are automatically translated
> + using the section header string table.
> +
> + """
> + if self._shdr_num == 0:
> + return
> +
> + size = self.ehdr.e_shentsize
> + if size != self.Shdr.size:
> + raise ValueError('Unexpected Shdr size in ELF header: {} != {}'
> + .format(size, self.Shdr.size))
> +
> + offset = self.ehdr.e_shoff
> + for _ in range(self._shdr_num):
> + shdr = self.read(self.Shdr, offset)
> + if resolve:
> + shdr = shdr.resolve(self._shdr_strtab)
> + yield shdr
> + offset += size
OK.
> +
> + def dynamic(self) -> Dyn:
> + """Generator iterating over the dynamic segment."""
> + for phdr in self.phdrs():
> + if phdr.p_type == Pt.PT_DYNAMIC:
> + # Pick the first dynamic segment, like the loader.
> + if phdr.p_filesz == 0:
> + # Probably separated debuginfo.
> + return
> + offset = phdr.p_offset
> + end = offset + phdr.p_memsz
> + size = self.Dyn.size
> + while True:
> + next_offset = offset + size
> + if next_offset > end:
> + raise ValueError(
> + 'Dynamic segment size {} is not a multiple of Dyn size {}'.format(
> + phdr.p_memsz, size))
> + yield self.read(self.Dyn, offset)
> + if next_offset == end:
> + return
> + offset = next_offset
OK.
> +
> + def syms(self, shdr: Shdr, resolve: bool=True) -> Sym:
> + """A generator iterating over a symbol table.
> +
> + If resolve, symbol names are automatically translated using
> + the string table for the symbol table.
> +
> + """
> + assert shdr.sh_type == Sht.SHT_SYMTAB
> + size = shdr.sh_entsize
> + if size != self.Sym.size:
> + raise ValueError('Invalid symbol table entry size {}'.format(size))
> + offset = shdr.sh_offset
> + end = shdr.sh_offset + shdr.sh_size
> + if resolve:
> + strtab = self._find_stringtab(shdr.sh_link)
> + while offset < end:
> + sym = self.read(self.Sym, offset)
> + if resolve:
> + sym = sym.resolve(strtab)
> + yield sym
> + offset += size
> + if offset != end:
> + raise ValueError('Symbol table is not a multiple of entry size')
OK.
> +
> + def lookup_string(self, strtab_index: int, strtab_offset: int) -> bytes:
> + """Looks up a string in a string table identified by its link index."""
> + try:
> + strtab = self._stringtab[strtab_index]
> + except KeyError:
> + strtab = self._find_stringtab(strtab_index)
> + return strtab.get(strtab_offset)
OK.
> +
> + def find_section(self, shndx: Shn) -> Shdr:
> + """Returns the section header for the indexed section.
> +
> + The section name is not resolved.
> + """
> + try:
> + return self._section[shndx]
> + except KeyError:
> + pass
> + if shndx in Shn:
> + raise ValueError('Reserved section index {}'.format(shndx))
> + idx = shndx.value
> + if idx < 0 or idx > self._shdr_num:
> + raise ValueError('Section index {} out of range [0, {})'.format(
> + idx, self._shdr_num))
> + shdr = self.read(
> + self.Shdr, self.ehdr.e_shoff + idx * self.Shdr.size)
> + self._section[shndx] = shdr
> + return shdr
OK.
> +
> + def _find_stringtab(self, sh_link: int) -> StringTable:
> + if sh_link in self._stringtab:
> + return self._stringtab
> + if sh_link < 0 or sh_link >= self._shdr_num:
> + raise ValueError('Section index {} out of range [0, {})'.format(
> + sh_link, self._shdr_num))
> + shdr = self.read(
> + self.Shdr, self.ehdr.e_shoff + sh_link * self.Shdr.size)
> + if shdr.sh_type != Sht.SHT_STRTAB:
> + raise ValueError(
> + 'Section {} is not a string table: {}'.format(
> + sh_link, shdr.sh_type))
> + strtab = StringTable(
> + self.image[shdr.sh_offset:shdr.sh_offset + shdr.sh_size])
> + # This could retrain essentially arbitrary amounts of data,
> + # but caching string tables seems important for performance.
> + self._stringtab[sh_link] = strtab
> + return strtab
> +
> +
> +__all__ = [name for name in dir() if name[0].isupper()]
OK.
>
> base-commit: 1a85970f41ea1e5abe6da2298a5e8fedcea26b70
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/2] scripts: Add glibcelf.py module
2022-04-21 15:54 ` [PATCH v3 1/2] scripts: Add glibcelf.py module Siddhesh Poyarekar
@ 2022-04-21 20:23 ` Florian Weimer
0 siblings, 0 replies; 26+ messages in thread
From: Florian Weimer @ 2022-04-21 20:23 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: libc-alpha
* Siddhesh Poyarekar:
> Could we generate all these from elf.h, or generate both, this and
> elf.h from some common source? At the moment there are a lot of
> platform-specific macros missing and some common ones too,
> e.g. DT_BIND_NOW. Further, STB_WEAK appears to have a different value
> from that in elf.h and there's an SHF_RETAIN that doesn't have a
> corresponding macro in elf.h.
I'm going to post a new patch with a consistency check against <elf.h>.
We could benefit from type information in glibcelf that is currently
missing from <elf.h>, I think, so synthesizing from <elf.h> directly
isn't quite possible.
I'm not sure yet how regular the mappings are, e.g. if we use any of the
*_HP_* constants (specific to HP/UX) for our parisc port. Maybe once
this is sorted out, we can come up with a common ancestor as a data
source.
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-11 15:32 ` [PATCH v3 2/2] Default to --with-default-link=no (bug 25812) Florian Weimer
@ 2022-04-22 6:12 ` Siddhesh Poyarekar
2022-04-22 6:35 ` Florian Weimer
2022-04-28 7:25 ` Fangrui Song
2022-04-29 12:59 ` Adhemerval Zanella
2 siblings, 1 reply; 26+ messages in thread
From: Siddhesh Poyarekar @ 2022-04-22 6:12 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 11/04/2022 21:02, Florian Weimer via Libc-alpha wrote:
> This is necessary to place the libio vtables into the RELRO segment.
> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
> verify that this is what actually happens.
>
> The new tests fail on ia64 due to lack of (default) RELRO support
> inbutils, so they are XFAILed there.
> ---
> v3: Fix INSTALL type and update wording.
> INSTALL | 6 ++
> configure | 65 +-----------
> configure.ac | 55 +----------
> elf/Makefile | 33 +++++++
> elf/tst-relro-symbols.py | 137 ++++++++++++++++++++++++++
> manual/install.texi | 6 ++
> sysdeps/unix/sysv/linux/ia64/Makefile | 6 ++
> 7 files changed, 190 insertions(+), 118 deletions(-)
> create mode 100644 elf/tst-relro-symbols.py
>
> diff --git a/INSTALL b/INSTALL
> index 63c022d6b9..de150f66eb 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -90,6 +90,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
> library will still be usable, but functionality may be lost--for
> example, you can't build a shared libc with old binutils.
>
> +'--with-default-link=FLAG'
> + With '--with-default-link=yes', the GNU C Library does not use a
> + custom linker scipt for linking its individual shared objects. The
> + default for FLAG is the opposite, 'no', because the custom linker
> + script is needed for full RELRO protection.
> +
Andreas' comments still apply here I think, i.e. fix the "scipt" type
and rephrase so that it's clearer that the option controls the library
build process and not the library itself.
Does it make sense to make this --disable-custom-link or
--enable-default-link instead, since the option is binary? The --with
prefix is typically for richer options, e.g. paths. Suggest something
like this:
--disable-custom-link
Don't use a custom linker script to build the GNU C Library,
preferring the static linker's default script instead. The custom
linker script is needed for full RELRO protection.
> '--with-nonshared-cflags=CFLAGS'
> Use additional compiler flags CFLAGS to build the parts of the
> library which are always statically linked into applications and
> diff --git a/configure b/configure
> index d2f413d05d..650bfd982c 100755
> --- a/configure
> +++ b/configure
> @@ -3375,7 +3375,7 @@ fi
> if test "${with_default_link+set}" = set; then :
> withval=$with_default_link; use_default_link=$withval
> else
> - use_default_link=default
> + use_default_link=no
> fi
... and then maybe adjust variable names accordingly.
>
>
> @@ -6184,69 +6184,6 @@ fi
> $as_echo "$libc_cv_hashstyle" >&6; }
>
>
> -# The linker's default -shared behavior is good enough if it
> -# does these things that our custom linker scripts ensure that
> -# all allocated NOTE sections come first.
> -if test "$use_default_link" = default; then
> - { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sufficient default -shared layout" >&5
> -$as_echo_n "checking for sufficient default -shared layout... " >&6; }
> -if ${libc_cv_use_default_link+:} false; then :
> - $as_echo_n "(cached) " >&6
> -else
> - libc_cv_use_default_link=no
> - cat > conftest.s <<\EOF
> - .section .note.a,"a",%note
> - .balign 4
> - .long 4,4,9
> - .string "GNU"
> - .string "foo"
> - .section .note.b,"a",%note
> - .balign 4
> - .long 4,4,9
> - .string "GNU"
> - .string "bar"
> -EOF
> - if { ac_try=' ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&5'
> - { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> - (eval $ac_try) 2>&5
> - ac_status=$?
> - $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> - test $ac_status = 0; }; } &&
> - ac_try=`$READELF -S conftest.so | sed -n \
> - '${x;p;}
> - s/^ *\[ *[1-9][0-9]*\] *\([^ ][^ ]*\) *\([^ ][^ ]*\) .*$/\2 \1/
> - t a
> - b
> - : a
> - H'`
> - then
> - libc_seen_a=no libc_seen_b=no
> - set -- $ac_try
> - while test $# -ge 2 -a "$1" = NOTE; do
> - case "$2" in
> - .note.a) libc_seen_a=yes ;;
> - .note.b) libc_seen_b=yes ;;
> - esac
> - shift 2
> - done
> - case "$libc_seen_a$libc_seen_b" in
> - yesyes)
> - libc_cv_use_default_link=yes
> - ;;
> - *)
> - echo >&5 "\
> -$libc_seen_a$libc_seen_b from:
> -$ac_try"
> - ;;
> - esac
> - fi
> - rm -f conftest*
> -fi
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_use_default_link" >&5
> -$as_echo "$libc_cv_use_default_link" >&6; }
> - use_default_link=$libc_cv_use_default_link
> -fi
> -
> { $as_echo "$as_me:${as_lineno-$LINENO}: checking for GLOB_DAT reloc" >&5
> $as_echo_n "checking for GLOB_DAT reloc... " >&6; }
> if ${libc_cv_has_glob_dat+:} false; then :
> diff --git a/configure.ac b/configure.ac
> index b6a747dece..605efd549d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -153,7 +153,7 @@ AC_ARG_WITH([default-link],
> AS_HELP_STRING([--with-default-link],
> [do not use explicit linker scripts]),
> [use_default_link=$withval],
> - [use_default_link=default])
> + [use_default_link=no])
>
> dnl Additional build flags injection.
> AC_ARG_WITH([nonshared-cflags],
> @@ -1371,59 +1371,6 @@ fi
> rm -f conftest*])
> AC_SUBST(libc_cv_hashstyle)
>
> -# The linker's default -shared behavior is good enough if it
> -# does these things that our custom linker scripts ensure that
> -# all allocated NOTE sections come first.
> -if test "$use_default_link" = default; then
> - AC_CACHE_CHECK([for sufficient default -shared layout],
> - libc_cv_use_default_link, [dnl
> - libc_cv_use_default_link=no
> - cat > conftest.s <<\EOF
> - .section .note.a,"a",%note
> - .balign 4
> - .long 4,4,9
> - .string "GNU"
> - .string "foo"
> - .section .note.b,"a",%note
> - .balign 4
> - .long 4,4,9
> - .string "GNU"
> - .string "bar"
> -EOF
> - if AC_TRY_COMMAND([dnl
> - ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&AS_MESSAGE_LOG_FD]) &&
> - ac_try=`$READELF -S conftest.so | sed -n \
> - ['${x;p;}
> - s/^ *\[ *[1-9][0-9]*\] *\([^ ][^ ]*\) *\([^ ][^ ]*\) .*$/\2 \1/
> - t a
> - b
> - : a
> - H']`
> - then
> - libc_seen_a=no libc_seen_b=no
> - set -- $ac_try
> - while test $# -ge 2 -a "$1" = NOTE; do
> - case "$2" in
> - .note.a) libc_seen_a=yes ;;
> - .note.b) libc_seen_b=yes ;;
> - esac
> - shift 2
> - done
> - case "$libc_seen_a$libc_seen_b" in
> - yesyes)
> - libc_cv_use_default_link=yes
> - ;;
> - *)
> - echo >&AS_MESSAGE_LOG_FD "\
> -$libc_seen_a$libc_seen_b from:
> -$ac_try"
> - ;;
> - esac
> - fi
> - rm -f conftest*])
> - use_default_link=$libc_cv_use_default_link
> -fi
> -
> AC_CACHE_CHECK(for GLOB_DAT reloc,
> libc_cv_has_glob_dat, [dnl
> cat > conftest.c <<EOF
> diff --git a/elf/Makefile b/elf/Makefile
> index c96924e9c2..366559fc96 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -543,6 +543,39 @@ endif
> endif
> endif
>
> +tests-special += $(objpfx)tst-relro-ldso.out $(objpfx)tst-relro-libc.out
> +$(objpfx)tst-relro-ldso.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
> + $(objpfx)ld.so
> + $(PYTHON) tst-relro-symbols.py $(objpfx)ld.so \
> + --required=_rtld_global_ro \
> + > $@ 2>&1; $(evaluate-test)
> +# The optional symbols are present in libc only if the architecture has
> +# the GLIBC_2.0 symbol set in libc.
> +$(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
> + $(common-objpfx)libc.so
> + $(PYTHON) tst-relro-symbols.py $(common-objpfx)libc.so \
> + --required=_IO_cookie_jumps \
> + --required=_IO_file_jumps \
> + --required=_IO_file_jumps_maybe_mmap \
> + --required=_IO_file_jumps_mmap \
> + --required=_IO_helper_jumps \
> + --required=_IO_mem_jumps \
> + --required=_IO_obstack_jumps \
> + --required=_IO_proc_jumps \
> + --required=_IO_str_chk_jumps \
> + --required=_IO_str_jumps \
> + --required=_IO_strn_jumps \
> + --required=_IO_wfile_jumps \
> + --required=_IO_wfile_jumps_maybe_mmap \
> + --required=_IO_wfile_jumps_mmap \
> + --required=_IO_wmem_jumps \
> + --required=_IO_wstr_jumps \
> + --required=_IO_wstrn_jumps \
> + --optional=_IO_old_cookie_jumps \
> + --optional=_IO_old_file_jumps \
> + --optional=_IO_old_proc_jumps \
> + > $@ 2>&1; $(evaluate-test)
> +
Run the new test. OK. Verified that it passes with
--with-default-link=no and fails with --with-default-link=yes.
> ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-valgrind-smoke.out
> endif
> diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
> new file mode 100644
> index 0000000000..368ea3349f
> --- /dev/null
> +++ b/elf/tst-relro-symbols.py
> @@ -0,0 +1,137 @@
> +#!/usr/bin/python3
> +# Verify that certain symbols are covered by RELRO.
> +# Copyright (C) 2022 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +#
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
> +
> +"""Analyze a (shared) object to verify that certain symbols are
> +present and covered by the PT_GNU_RELRO segment.
> +
> +"""
> +
> +import argparse
> +import os.path
> +import sys
> +
> +# Make available glibc Python modules.
> +sys.path.append(os.path.join(
> + os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
I wonder if we should put this in the python environment for all glibc
python scripts. Only a soft suggestion though since this is just the
first module usage. Maybe as and when we port other such tests to this
module we should rethink this.
> +
> +import glibcelf
> +
> +def find_relro(path: str, img: glibcelf.Image) -> (int, int):
> + """Discover the address range of the PT_GNU_RELRO segment."""
> + for phdr in img.phdrs():
> + if phdr.p_type == glibcelf.Pt.PT_GNU_RELRO:
> + # The computation is not entirely accurate because
> + # _dl_protect_relro in elf/dl-reloc.c rounds both the
> + # start end and downwards using the run-time page size.
> + return phdr.p_vaddr, phdr.p_vaddr + phdr.p_memsz
> + sys.stdout.write('{}: error: no PT_GNU_RELRO segment\n'.format(path))
> + sys.exit(1)
OK.
> +
> +def check_in_relro(kind, relro_begin, relro_end, name, start, size, error):
> + """Check if a section or symbol falls within in the RELRO segment."""
> + end = start + size - 1
> + if not (relro_begin <= start < end < relro_end):
> + error(
> + '{} {!r} of size {} at 0x{:x} is not in RELRO range [0x{:x}, 0x{:x})'.format(
> + kind, name.decode('UTF-8'), start, size,
> + relro_begin, relro_end))
OK. Some day this will be a reusable function I think; we may end up
with more tests that want to check if some symbol is in relro.
> +
> +def get_parser():
> + """Return an argument parser for this script."""
> + parser = argparse.ArgumentParser(description=__doc__)
> + parser.add_argument('object', help='path to object file to check')
> + parser.add_argument('--required', metavar='NAME', default=(),
> + help='required symbol names', nargs='*')
> + parser.add_argument('--optional', metavar='NAME', default=(),
> + help='required symbol names', nargs='*')
> + return parser
OK.
> +
> +def main(argv):
> + """The main entry point."""
> + parser = get_parser()
> + opts = parser.parse_args(argv)
> + img = glibcelf.Image.readfile(opts.object)
> +
> + required_symbols = frozenset([sym.encode('UTF-8')
> + for sym in opts.required])
> + optional_symbols = frozenset([sym.encode('UTF-8')
> + for sym in opts.optional])
> + check_symbols = required_symbols | optional_symbols
> +
> + # Tracks the symbols in check_symbols that have been found.
> + symbols_found = set()
> +
> + # Discover the extent of the RELRO segment.
> + relro_begin, relro_end = find_relro(opts.object, img)
> + symbol_table_found = False
> +
> + errors = False
> + def error(msg: str) -> None:
> + """Record an error condition and write a message to standard output."""
> + nonlocal errors
> + errors = True
> + sys.stdout.write('{}: error: {}\n'.format(opts.object, msg))
> +
> + # Iterate over section headers to find the symbol table.
> + for shdr in img.shdrs():
> + if shdr.sh_type == glibcelf.Sht.SHT_SYMTAB:
> + symbol_table_found = True
> + for sym in img.syms(shdr):
> + if sym.st_name in check_symbols:
> + symbols_found.add(sym.st_name)
> +
> + # Validate symbol type, section, and size.
> + if sym.st_info.type != glibcelf.Stt.STT_OBJECT:
> + error('symbol {!r} has wrong type {}'.format(
> + sym.st_name.decode('UTF-8'), sym.st_info.type))
> + if sym.st_shndx in glibcelf.Shn:
> + error('symbol {!r} has reserved section {}'.format(
> + sym.st_name.decode('UTF-8'), sym.st_shndx))
> + continue
> + if sym.st_size == 0:
> + error('symbol {!r} has size zero'.format(
> + sym.st_name.decode('UTF-8')))
> + continue
> +
> + check_in_relro('symbol', relro_begin, relro_end,
> + sym.st_name, sym.st_value, sym.st_size,
> + error)
> + continue # SHT_SYMTAB
> + if shdr.sh_name == b'.data.rel.ro' \
> + or shdr.sh_name.startswith(b'.data.rel.ro.'):
> + check_in_relro('section', relro_begin, relro_end,
> + shdr.sh_name, shdr.sh_addr, shdr.sh_size,
> + error)
> + continue
> +
> + if required_symbols - symbols_found:
> + for sym in sorted(required_symbols - symbols_found):
> + error('symbol {!r} not found'.format(sym.decode('UTF-8')))
> +
> + if errors:
> + sys.exit(1)
> +
> + if not symbol_table_found:
> + sys.stdout.write(
> + '{}: warning: no symbol table found (stripped object)\n'.format(
> + opts.object))
> + sys.exit(77)
> +
> +if __name__ == '__main__':
> + main(sys.argv[1:])
OK.
> diff --git a/manual/install.texi b/manual/install.texi
> index 29c52f2927..fcfb6901e4 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -117,6 +117,12 @@ problem and suppress these constructs, so that the library will still be
> usable, but functionality may be lost---for example, you can't build a
> shared libc with old binutils.
>
> +@item --with-default-link=@var{FLAG}
> +With @code{--with-default-link=yes}, the build system does not use a
> +custom linker script for linking shared objects. The default for
> +@var{FLAG} is the opposite, @samp{no}, because the custom linker script
> +is needed for full RELRO protection.
> +
> @item --with-nonshared-cflags=@var{cflags}
> Use additional compiler flags @var{cflags} to build the parts of the
> library which are always statically linked into applications and
> diff --git a/sysdeps/unix/sysv/linux/ia64/Makefile b/sysdeps/unix/sysv/linux/ia64/Makefile
> index da85ba43e2..c5cc41b367 100644
> --- a/sysdeps/unix/sysv/linux/ia64/Makefile
> +++ b/sysdeps/unix/sysv/linux/ia64/Makefile
> @@ -1,3 +1,9 @@
> +ifeq ($(subdir),elf)
> +# ia64 does not support PT_GNU_RELRO.
> +test-xfail-tst-relro-ldso = yes
> +test-xfail-tst-relro-libc = yes
> +endif
> +
> ifeq ($(subdir),misc)
> sysdep_headers += sys/rse.h
> endif
OK.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-22 6:12 ` Siddhesh Poyarekar
@ 2022-04-22 6:35 ` Florian Weimer
2022-04-22 8:12 ` Siddhesh Poyarekar
` (2 more replies)
0 siblings, 3 replies; 26+ messages in thread
From: Florian Weimer @ 2022-04-22 6:35 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: libc-alpha
* Siddhesh Poyarekar:
>> diff --git a/INSTALL b/INSTALL
>> index 63c022d6b9..de150f66eb 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -90,6 +90,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
>> library will still be usable, but functionality may be lost--for
>> example, you can't build a shared libc with old binutils.
>> +'--with-default-link=FLAG'
>> + With '--with-default-link=yes', the GNU C Library does not use a
>> + custom linker scipt for linking its individual shared objects. The
>> + default for FLAG is the opposite, 'no', because the custom linker
>> + script is needed for full RELRO protection.
>> +
>
> Andreas' comments still apply here I think, i.e. fix the "scipt" type
> and rephrase so that it's clearer that the option controls the library
> build process and not the library itself.
I thought I had fixed this. What about this?
'--with-default-link=FLAG'
With '--with-default-link=yes', the build system does not use a
custom linker script for linking shared objects. The default for
FLAG is the opposite, 'no', because the custom linker script is
needed for full RELRO protection.
> Does it make sense to make this --disable-custom-link or
> --enable-default-link instead, since the option is binary? The --with
> prefix is typically for richer options, e.g. paths. Suggest something
> like this:
>
> --disable-custom-link
> Don't use a custom linker script to build the GNU C Library,
> preferring the static linker's default script instead. The custom
> linker script is needed for full RELRO protection.
I want to backport this, and distributions are already using this
option, so I prefer not to make changes here.
>> diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
>> new file mode 100644
>> index 0000000000..368ea3349f
>> --- /dev/null
>> +++ b/elf/tst-relro-symbols.py
>> @@ -0,0 +1,137 @@
>> +# Make available glibc Python modules.
>> +sys.path.append(os.path.join(
>> + os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
>
> I wonder if we should put this in the python environment for all glibc
> python scripts. Only a soft suggestion though since this is just the
> first module usage. Maybe as and when we port other such tests to
> this module we should rethink this.
We set PYTHONPATH in the makefiles in many cases. I did this mostly for
development.
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-22 6:35 ` Florian Weimer
@ 2022-04-22 8:12 ` Siddhesh Poyarekar
2022-04-22 8:24 ` Florian Weimer
2022-04-22 8:56 ` Andreas Schwab
2022-04-29 16:02 ` Dmitry V. Levin
2 siblings, 1 reply; 26+ messages in thread
From: Siddhesh Poyarekar @ 2022-04-22 8:12 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 22/04/2022 12:05, Florian Weimer wrote:
> * Siddhesh Poyarekar:
>
>>> diff --git a/INSTALL b/INSTALL
>>> index 63c022d6b9..de150f66eb 100644
>>> --- a/INSTALL
>>> +++ b/INSTALL
>>> @@ -90,6 +90,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
>>> library will still be usable, but functionality may be lost--for
>>> example, you can't build a shared libc with old binutils.
>>> +'--with-default-link=FLAG'
>>> + With '--with-default-link=yes', the GNU C Library does not use a
>>> + custom linker scipt for linking its individual shared objects. The
>>> + default for FLAG is the opposite, 'no', because the custom linker
>>> + script is needed for full RELRO protection.
>>> +
>>
>> Andreas' comments still apply here I think, i.e. fix the "scipt" type
>> and rephrase so that it's clearer that the option controls the library
>> build process and not the library itself.
>
> I thought I had fixed this. What about this?
>
> '--with-default-link=FLAG'
> With '--with-default-link=yes', the build system does not use a
> custom linker script for linking shared objects. The default for
> FLAG is the opposite, 'no', because the custom linker script is
> needed for full RELRO protection.
That looks better, thanks.
>> Does it make sense to make this --disable-custom-link or
>> --enable-default-link instead, since the option is binary? The --with
>> prefix is typically for richer options, e.g. paths. Suggest something
>> like this:
>>
>> --disable-custom-link
>> Don't use a custom linker script to build the GNU C Library,
>> preferring the static linker's default script instead. The custom
>> linker script is needed for full RELRO protection.
>
> I want to backport this, and distributions are already using this
> option, so I prefer not to make changes here.
Fair enough.
>>> diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
>>> new file mode 100644
>>> index 0000000000..368ea3349f
>>> --- /dev/null
>>> +++ b/elf/tst-relro-symbols.py
>>> @@ -0,0 +1,137 @@
>
>>> +# Make available glibc Python modules.
>>> +sys.path.append(os.path.join(
>>> + os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
>>
>> I wonder if we should put this in the python environment for all glibc
>> python scripts. Only a soft suggestion though since this is just the
>> first module usage. Maybe as and when we port other such tests to
>> this module we should rethink this.
>
> We set PYTHONPATH in the makefiles in many cases. I did this mostly for
> development.
Indeed, in fact we have py-env in Rules which may be helpful. How about
using it the invocation instead of the above then?
Thanks,
Siddhesh
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-22 8:12 ` Siddhesh Poyarekar
@ 2022-04-22 8:24 ` Florian Weimer
2022-04-22 8:32 ` Siddhesh Poyarekar
0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2022-04-22 8:24 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: libc-alpha
* Siddhesh Poyarekar:
>>> I wonder if we should put this in the python environment for all glibc
>>> python scripts. Only a soft suggestion though since this is just the
>>> first module usage. Maybe as and when we port other such tests to
>>> this module we should rethink this.
>> We set PYTHONPATH in the makefiles in many cases. I did this mostly
>> for development.
>
> Indeed, in fact we have py-env in Rules which may be helpful. How
> about using it the invocation instead of the above then?
I do like that I can run the script directly. 8-)
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-22 8:24 ` Florian Weimer
@ 2022-04-22 8:32 ` Siddhesh Poyarekar
2022-04-22 8:34 ` Florian Weimer
0 siblings, 1 reply; 26+ messages in thread
From: Siddhesh Poyarekar @ 2022-04-22 8:32 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 22/04/2022 13:54, Florian Weimer wrote:
> * Siddhesh Poyarekar:
>
>>>> I wonder if we should put this in the python environment for all glibc
>>>> python scripts. Only a soft suggestion though since this is just the
>>>> first module usage. Maybe as and when we port other such tests to
>>>> this module we should rethink this.
>
>>> We set PYTHONPATH in the makefiles in many cases. I did this mostly
>>> for development.
>>
>> Indeed, in fact we have py-env in Rules which may be helpful. How
>> about using it the invocation instead of the above then?
>
> I do like that I can run the script directly. 8-)
Fair enough :)
Siddhesh
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-22 8:32 ` Siddhesh Poyarekar
@ 2022-04-22 8:34 ` Florian Weimer
2022-04-22 8:36 ` Siddhesh Poyarekar
0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2022-04-22 8:34 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: libc-alpha
* Siddhesh Poyarekar:
> On 22/04/2022 13:54, Florian Weimer wrote:
>> * Siddhesh Poyarekar:
>>
>>>>> I wonder if we should put this in the python environment for all glibc
>>>>> python scripts. Only a soft suggestion though since this is just the
>>>>> first module usage. Maybe as and when we port other such tests to
>>>>> this module we should rethink this.
>>
>>>> We set PYTHONPATH in the makefiles in many cases. I did this mostly
>>>> for development.
>>>
>>> Indeed, in fact we have py-env in Rules which may be helpful. How
>>> about using it the invocation instead of the above then?
>> I do like that I can run the script directly. 8-)
>
> Fair enough :)
So it's good to install along with the glibcelf.py patch?
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-22 8:34 ` Florian Weimer
@ 2022-04-22 8:36 ` Siddhesh Poyarekar
0 siblings, 0 replies; 26+ messages in thread
From: Siddhesh Poyarekar @ 2022-04-22 8:36 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 22/04/2022 14:04, Florian Weimer wrote:
> * Siddhesh Poyarekar:
>
>> On 22/04/2022 13:54, Florian Weimer wrote:
>>> * Siddhesh Poyarekar:
>>>
>>>>>> I wonder if we should put this in the python environment for all glibc
>>>>>> python scripts. Only a soft suggestion though since this is just the
>>>>>> first module usage. Maybe as and when we port other such tests to
>>>>>> this module we should rethink this.
>>>
>>>>> We set PYTHONPATH in the makefiles in many cases. I did this mostly
>>>>> for development.
>>>>
>>>> Indeed, in fact we have py-env in Rules which may be helpful. How
>>>> about using it the invocation instead of the above then?
>>> I do like that I can run the script directly. 8-)
>>
>> Fair enough :)
>
> So it's good to install along with the glibcelf.py patch?
Yes please.
Thanks,
Siddhesh
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-22 6:35 ` Florian Weimer
2022-04-22 8:12 ` Siddhesh Poyarekar
@ 2022-04-22 8:56 ` Andreas Schwab
2022-04-29 16:02 ` Dmitry V. Levin
2 siblings, 0 replies; 26+ messages in thread
From: Andreas Schwab @ 2022-04-22 8:56 UTC (permalink / raw)
To: Florian Weimer via Libc-alpha; +Cc: Siddhesh Poyarekar, Florian Weimer
On Apr 22 2022, Florian Weimer via Libc-alpha wrote:
> '--with-default-link=FLAG'
> With '--with-default-link=yes', the build system does not use a
> custom linker script for linking shared objects. The default for
> FLAG is the opposite, 'no', because the custom linker script is
> needed for full RELRO protection.
Since it's a binary option, just use --with-default-link and
--without-default-link.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-11 15:32 ` [PATCH v3 2/2] Default to --with-default-link=no (bug 25812) Florian Weimer
2022-04-22 6:12 ` Siddhesh Poyarekar
@ 2022-04-28 7:25 ` Fangrui Song
2022-04-28 8:40 ` Florian Weimer
2022-04-29 12:59 ` Adhemerval Zanella
2 siblings, 1 reply; 26+ messages in thread
From: Fangrui Song @ 2022-04-28 7:25 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 2022-04-11, Florian Weimer via Libc-alpha wrote:
>This is necessary to place the libio vtables into the RELRO segment.
>New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
>verify that this is what actually happens.
>
>The new tests fail on ia64 due to lack of (default) RELRO support
>inbutils, so they are XFAILed there.
>---
>v3: Fix INSTALL type and update wording.
> INSTALL | 6 ++
> configure | 65 +-----------
> configure.ac | 55 +----------
> elf/Makefile | 33 +++++++
> elf/tst-relro-symbols.py | 137 ++++++++++++++++++++++++++
> manual/install.texi | 6 ++
> sysdeps/unix/sysv/linux/ia64/Makefile | 6 ++
> 7 files changed, 190 insertions(+), 118 deletions(-)
> create mode 100644 elf/tst-relro-symbols.py
>
>diff --git a/INSTALL b/INSTALL
>index 63c022d6b9..de150f66eb 100644
>--- a/INSTALL
>+++ b/INSTALL
>@@ -90,6 +90,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
> library will still be usable, but functionality may be lost--for
> example, you can't build a shared libc with old binutils.
>
>+'--with-default-link=FLAG'
>+ With '--with-default-link=yes', the GNU C Library does not use a
>+ custom linker scipt for linking its individual shared objects. The
>+ default for FLAG is the opposite, 'no', because the custom linker
>+ script is needed for full RELRO protection.
>+
> '--with-nonshared-cflags=CFLAGS'
> Use additional compiler flags CFLAGS to build the parts of the
> library which are always statically linked into applications and
>diff --git a/configure b/configure
>index d2f413d05d..650bfd982c 100755
>--- a/configure
>+++ b/configure
>@@ -3375,7 +3375,7 @@ fi
> if test "${with_default_link+set}" = set; then :
> withval=$with_default_link; use_default_link=$withval
> else
>- use_default_link=default
>+ use_default_link=no
> fi
>
>
>@@ -6184,69 +6184,6 @@ fi
> $as_echo "$libc_cv_hashstyle" >&6; }
>
>
>-# The linker's default -shared behavior is good enough if it
>-# does these things that our custom linker scripts ensure that
>-# all allocated NOTE sections come first.
>-if test "$use_default_link" = default; then
>- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sufficient default -shared layout" >&5
>-$as_echo_n "checking for sufficient default -shared layout... " >&6; }
>-if ${libc_cv_use_default_link+:} false; then :
>- $as_echo_n "(cached) " >&6
>-else
>- libc_cv_use_default_link=no
>- cat > conftest.s <<\EOF
>- .section .note.a,"a",%note
>- .balign 4
>- .long 4,4,9
>- .string "GNU"
>- .string "foo"
>- .section .note.b,"a",%note
>- .balign 4
>- .long 4,4,9
>- .string "GNU"
>- .string "bar"
>-EOF
>- if { ac_try=' ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&5'
>- { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>- (eval $ac_try) 2>&5
>- ac_status=$?
>- $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>- test $ac_status = 0; }; } &&
>- ac_try=`$READELF -S conftest.so | sed -n \
>- '${x;p;}
>- s/^ *\[ *[1-9][0-9]*\] *\([^ ][^ ]*\) *\([^ ][^ ]*\) .*$/\2 \1/
>- t a
>- b
>- : a
>- H'`
>- then
>- libc_seen_a=no libc_seen_b=no
>- set -- $ac_try
>- while test $# -ge 2 -a "$1" = NOTE; do
>- case "$2" in
>- .note.a) libc_seen_a=yes ;;
>- .note.b) libc_seen_b=yes ;;
>- esac
>- shift 2
>- done
>- case "$libc_seen_a$libc_seen_b" in
>- yesyes)
>- libc_cv_use_default_link=yes
>- ;;
>- *)
>- echo >&5 "\
>-$libc_seen_a$libc_seen_b from:
>-$ac_try"
>- ;;
>- esac
>- fi
>- rm -f conftest*
>-fi
>-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_use_default_link" >&5
>-$as_echo "$libc_cv_use_default_link" >&6; }
>- use_default_link=$libc_cv_use_default_link
>-fi
>-
> { $as_echo "$as_me:${as_lineno-$LINENO}: checking for GLOB_DAT reloc" >&5
> $as_echo_n "checking for GLOB_DAT reloc... " >&6; }
> if ${libc_cv_has_glob_dat+:} false; then :
>diff --git a/configure.ac b/configure.ac
>index b6a747dece..605efd549d 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -153,7 +153,7 @@ AC_ARG_WITH([default-link],
> AS_HELP_STRING([--with-default-link],
> [do not use explicit linker scripts]),
> [use_default_link=$withval],
>- [use_default_link=default])
>+ [use_default_link=no])
>
> dnl Additional build flags injection.
> AC_ARG_WITH([nonshared-cflags],
>@@ -1371,59 +1371,6 @@ fi
> rm -f conftest*])
> AC_SUBST(libc_cv_hashstyle)
>
>-# The linker's default -shared behavior is good enough if it
>-# does these things that our custom linker scripts ensure that
>-# all allocated NOTE sections come first.
>-if test "$use_default_link" = default; then
>- AC_CACHE_CHECK([for sufficient default -shared layout],
>- libc_cv_use_default_link, [dnl
>- libc_cv_use_default_link=no
>- cat > conftest.s <<\EOF
>- .section .note.a,"a",%note
>- .balign 4
>- .long 4,4,9
>- .string "GNU"
>- .string "foo"
>- .section .note.b,"a",%note
>- .balign 4
>- .long 4,4,9
>- .string "GNU"
>- .string "bar"
>-EOF
>- if AC_TRY_COMMAND([dnl
>- ${CC-cc} $ASFLAGS -shared -o conftest.so conftest.s 1>&AS_MESSAGE_LOG_FD]) &&
>- ac_try=`$READELF -S conftest.so | sed -n \
>- ['${x;p;}
>- s/^ *\[ *[1-9][0-9]*\] *\([^ ][^ ]*\) *\([^ ][^ ]*\) .*$/\2 \1/
>- t a
>- b
>- : a
>- H']`
>- then
>- libc_seen_a=no libc_seen_b=no
>- set -- $ac_try
>- while test $# -ge 2 -a "$1" = NOTE; do
>- case "$2" in
>- .note.a) libc_seen_a=yes ;;
>- .note.b) libc_seen_b=yes ;;
>- esac
>- shift 2
>- done
>- case "$libc_seen_a$libc_seen_b" in
>- yesyes)
>- libc_cv_use_default_link=yes
>- ;;
>- *)
>- echo >&AS_MESSAGE_LOG_FD "\
>-$libc_seen_a$libc_seen_b from:
>-$ac_try"
>- ;;
>- esac
>- fi
>- rm -f conftest*])
>- use_default_link=$libc_cv_use_default_link
>-fi
>-
> AC_CACHE_CHECK(for GLOB_DAT reloc,
> libc_cv_has_glob_dat, [dnl
> cat > conftest.c <<EOF
>diff --git a/elf/Makefile b/elf/Makefile
>index c96924e9c2..366559fc96 100644
>--- a/elf/Makefile
>+++ b/elf/Makefile
>@@ -543,6 +543,39 @@ endif
> endif
> endif
>
>+tests-special += $(objpfx)tst-relro-ldso.out $(objpfx)tst-relro-libc.out
>+$(objpfx)tst-relro-ldso.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
>+ $(objpfx)ld.so
>+ $(PYTHON) tst-relro-symbols.py $(objpfx)ld.so \
>+ --required=_rtld_global_ro \
>+ > $@ 2>&1; $(evaluate-test)
>+# The optional symbols are present in libc only if the architecture has
>+# the GLIBC_2.0 symbol set in libc.
>+$(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
>+ $(common-objpfx)libc.so
>+ $(PYTHON) tst-relro-symbols.py $(common-objpfx)libc.so \
>+ --required=_IO_cookie_jumps \
>+ --required=_IO_file_jumps \
>+ --required=_IO_file_jumps_maybe_mmap \
>+ --required=_IO_file_jumps_mmap \
>+ --required=_IO_helper_jumps \
>+ --required=_IO_mem_jumps \
>+ --required=_IO_obstack_jumps \
>+ --required=_IO_proc_jumps \
>+ --required=_IO_str_chk_jumps \
>+ --required=_IO_str_jumps \
>+ --required=_IO_strn_jumps \
>+ --required=_IO_wfile_jumps \
>+ --required=_IO_wfile_jumps_maybe_mmap \
>+ --required=_IO_wfile_jumps_mmap \
>+ --required=_IO_wmem_jumps \
>+ --required=_IO_wstr_jumps \
>+ --required=_IO_wstrn_jumps \
>+ --optional=_IO_old_cookie_jumps \
>+ --optional=_IO_old_file_jumps \
>+ --optional=_IO_old_proc_jumps \
>+ > $@ 2>&1; $(evaluate-test)
>+
> ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-valgrind-smoke.out
> endif
>diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
>new file mode 100644
>index 0000000000..368ea3349f
>--- /dev/null
>+++ b/elf/tst-relro-symbols.py
>@@ -0,0 +1,137 @@
>+#!/usr/bin/python3
>+# Verify that certain symbols are covered by RELRO.
>+# Copyright (C) 2022 Free Software Foundation, Inc.
>+# This file is part of the GNU C Library.
>+#
>+# The GNU C Library is free software; you can redistribute it and/or
>+# modify it under the terms of the GNU Lesser General Public
>+# License as published by the Free Software Foundation; either
>+# version 2.1 of the License, or (at your option) any later version.
>+#
>+# The GNU C Library is distributed in the hope that it will be useful,
>+# but WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+# Lesser General Public License for more details.
>+#
>+# You should have received a copy of the GNU Lesser General Public
>+# License along with the GNU C Library; if not, see
>+# <https://www.gnu.org/licenses/>.
>+
>+"""Analyze a (shared) object to verify that certain symbols are
>+present and covered by the PT_GNU_RELRO segment.
>+
>+"""
>+
>+import argparse
>+import os.path
>+import sys
>+
>+# Make available glibc Python modules.
>+sys.path.append(os.path.join(
>+ os.path.dirname(os.path.realpath(__file__)), os.path.pardir, 'scripts'))
>+
>+import glibcelf
>+
>+def find_relro(path: str, img: glibcelf.Image) -> (int, int):
>+ """Discover the address range of the PT_GNU_RELRO segment."""
>+ for phdr in img.phdrs():
>+ if phdr.p_type == glibcelf.Pt.PT_GNU_RELRO:
>+ # The computation is not entirely accurate because
>+ # _dl_protect_relro in elf/dl-reloc.c rounds both the
>+ # start end and downwards using the run-time page size.
>+ return phdr.p_vaddr, phdr.p_vaddr + phdr.p_memsz
>+ sys.stdout.write('{}: error: no PT_GNU_RELRO segment\n'.format(path))
>+ sys.exit(1)
>+
>+def check_in_relro(kind, relro_begin, relro_end, name, start, size, error):
>+ """Check if a section or symbol falls within in the RELRO segment."""
>+ end = start + size - 1
>+ if not (relro_begin <= start < end < relro_end):
>+ error(
>+ '{} {!r} of size {} at 0x{:x} is not in RELRO range [0x{:x}, 0x{:x})'.format(
>+ kind, name.decode('UTF-8'), start, size,
>+ relro_begin, relro_end))
>+
>+def get_parser():
>+ """Return an argument parser for this script."""
>+ parser = argparse.ArgumentParser(description=__doc__)
>+ parser.add_argument('object', help='path to object file to check')
>+ parser.add_argument('--required', metavar='NAME', default=(),
>+ help='required symbol names', nargs='*')
>+ parser.add_argument('--optional', metavar='NAME', default=(),
>+ help='required symbol names', nargs='*')
>+ return parser
>+
>+def main(argv):
>+ """The main entry point."""
>+ parser = get_parser()
>+ opts = parser.parse_args(argv)
>+ img = glibcelf.Image.readfile(opts.object)
>+
>+ required_symbols = frozenset([sym.encode('UTF-8')
>+ for sym in opts.required])
>+ optional_symbols = frozenset([sym.encode('UTF-8')
>+ for sym in opts.optional])
>+ check_symbols = required_symbols | optional_symbols
>+
>+ # Tracks the symbols in check_symbols that have been found.
>+ symbols_found = set()
>+
>+ # Discover the extent of the RELRO segment.
>+ relro_begin, relro_end = find_relro(opts.object, img)
>+ symbol_table_found = False
>+
>+ errors = False
>+ def error(msg: str) -> None:
>+ """Record an error condition and write a message to standard output."""
>+ nonlocal errors
>+ errors = True
>+ sys.stdout.write('{}: error: {}\n'.format(opts.object, msg))
>+
>+ # Iterate over section headers to find the symbol table.
>+ for shdr in img.shdrs():
>+ if shdr.sh_type == glibcelf.Sht.SHT_SYMTAB:
>+ symbol_table_found = True
>+ for sym in img.syms(shdr):
>+ if sym.st_name in check_symbols:
>+ symbols_found.add(sym.st_name)
>+
>+ # Validate symbol type, section, and size.
>+ if sym.st_info.type != glibcelf.Stt.STT_OBJECT:
>+ error('symbol {!r} has wrong type {}'.format(
>+ sym.st_name.decode('UTF-8'), sym.st_info.type))
>+ if sym.st_shndx in glibcelf.Shn:
>+ error('symbol {!r} has reserved section {}'.format(
>+ sym.st_name.decode('UTF-8'), sym.st_shndx))
>+ continue
>+ if sym.st_size == 0:
>+ error('symbol {!r} has size zero'.format(
>+ sym.st_name.decode('UTF-8')))
>+ continue
>+
>+ check_in_relro('symbol', relro_begin, relro_end,
>+ sym.st_name, sym.st_value, sym.st_size,
>+ error)
>+ continue # SHT_SYMTAB
>+ if shdr.sh_name == b'.data.rel.ro' \
>+ or shdr.sh_name.startswith(b'.data.rel.ro.'):
>+ check_in_relro('section', relro_begin, relro_end,
>+ shdr.sh_name, shdr.sh_addr, shdr.sh_size,
>+ error)
>+ continue
>+
>+ if required_symbols - symbols_found:
>+ for sym in sorted(required_symbols - symbols_found):
>+ error('symbol {!r} not found'.format(sym.decode('UTF-8')))
>+
>+ if errors:
>+ sys.exit(1)
>+
>+ if not symbol_table_found:
>+ sys.stdout.write(
>+ '{}: warning: no symbol table found (stripped object)\n'.format(
>+ opts.object))
>+ sys.exit(77)
>+
>+if __name__ == '__main__':
>+ main(sys.argv[1:])
>diff --git a/manual/install.texi b/manual/install.texi
>index 29c52f2927..fcfb6901e4 100644
>--- a/manual/install.texi
>+++ b/manual/install.texi
>@@ -117,6 +117,12 @@ problem and suppress these constructs, so that the library will still be
> usable, but functionality may be lost---for example, you can't build a
> shared libc with old binutils.
>
>+@item --with-default-link=@var{FLAG}
>+With @code{--with-default-link=yes}, the build system does not use a
>+custom linker script for linking shared objects. The default for
>+@var{FLAG} is the opposite, @samp{no}, because the custom linker script
>+is needed for full RELRO protection.
>+
> @item --with-nonshared-cflags=@var{cflags}
> Use additional compiler flags @var{cflags} to build the parts of the
> library which are always statically linked into applications and
>diff --git a/sysdeps/unix/sysv/linux/ia64/Makefile b/sysdeps/unix/sysv/linux/ia64/Makefile
>index da85ba43e2..c5cc41b367 100644
>--- a/sysdeps/unix/sysv/linux/ia64/Makefile
>+++ b/sysdeps/unix/sysv/linux/ia64/Makefile
>@@ -1,3 +1,9 @@
>+ifeq ($(subdir),elf)
>+# ia64 does not support PT_GNU_RELRO.
>+test-xfail-tst-relro-ldso = yes
>+test-xfail-tst-relro-libc = yes
>+endif
>+
> ifeq ($(subdir),misc)
> sysdep_headers += sys/rse.h
> endif
>--
>2.35.1
>
I use chromium's prebuilt clang/lld and do something like:
curl -s https://raw.githubusercontent.com/chromium/chromium/main/tools/clang/scripts/update.py | python3 - --output-dir=~/Stable
sudo ln -sf ~/Stable/bin/lld /usr/local/bin/ld
so that gcc will use ld.lld.
ld.lld doesn't have a built-in linker script (ld.bfd --verbose), so $@T will be empty and `test -s $@T` will fail.
I wonder how to better support ld.lld with the default --with-default-link=no ...
Besides, the position of .hash in --with-default-link=no seems weird.
71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
of the RO segment.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-28 7:25 ` Fangrui Song
@ 2022-04-28 8:40 ` Florian Weimer
2022-04-29 6:00 ` Fangrui Song
0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2022-04-28 8:40 UTC (permalink / raw)
To: Fangrui Song; +Cc: libc-alpha
* Fangrui Song:
> I use chromium's prebuilt clang/lld and do something like:
>
> curl -s https://raw.githubusercontent.com/chromium/chromium/main/tools/clang/scripts/update.py | python3 - --output-dir=~/Stable
> sudo ln -sf ~/Stable/bin/lld /usr/local/bin/ld
>
> so that gcc will use ld.lld.
>
> ld.lld doesn't have a built-in linker script (ld.bfd --verbose), so $@T will be empty and `test -s $@T` will fail.
> I wonder how to better support ld.lld with the default --with-default-link=no ...
How does lld place sections without a linker script? Purely based on
names?
> Besides, the position of .hash in --with-default-link=no seems weird.
> 71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
> of the RO segment.
I think it's odd that we have an old-style hash table at all. Do we
really need it? All tools should have migrated by now.
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-28 8:40 ` Florian Weimer
@ 2022-04-29 6:00 ` Fangrui Song
2022-04-29 7:09 ` Florian Weimer
0 siblings, 1 reply; 26+ messages in thread
From: Fangrui Song @ 2022-04-29 6:00 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 2022-04-28, Florian Weimer wrote:
>* Fangrui Song:
>
>> I use chromium's prebuilt clang/lld and do something like:
>>
>> curl -s https://raw.githubusercontent.com/chromium/chromium/main/tools/clang/scripts/update.py | python3 - --output-dir=~/Stable
>> sudo ln -sf ~/Stable/bin/lld /usr/local/bin/ld
>>
>> so that gcc will use ld.lld.
>>
>> ld.lld doesn't have a built-in linker script (ld.bfd --verbose), so $@T will be empty and `test -s $@T` will fail.
>> I wonder how to better support ld.lld with the default --with-default-link=no ...
>
>How does lld place sections without a linker script? Purely based on
>names?
It entirely uses code to describe the built-in rules like gold, while
GNU ld uses an internal linker script plus built-in code.
(some rules cannot be described by the linker script language and needs
code anyway).
There is no support dumping lld/gold's built-in rules into a linker
script.
I will add DATA_SEGMENT_RELRO_END support to ld.lld:
https://reviews.llvm.org/D124656 . With the patch, ld.lld can read shlib.lds
transformed from ld.bfd --verbose output.
I can think of 3 ways for lld:
* ask lld users to use --with-default-link=yes
* remove `test -s $@T` (if lld)
* add -fuse-ld=bfd
>> Besides, the position of .hash in --with-default-link=no seems weird.
>> 71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
>> of the RO segment.
>
>I think it's odd that we have an old-style hash table at all. Do we
>really need it? All tools should have migrated by now.
We can remove .hash for non-mips architectures.
(I am a bit sad .MIPS.xhash support went in, but thankfully it doesn't take
too much code. The translation table could be solved in a better way. I
suspect the whole thing is unnecessary with RELR.)
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 6:00 ` Fangrui Song
@ 2022-04-29 7:09 ` Florian Weimer
2022-04-29 7:43 ` Fangrui Song
0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2022-04-29 7:09 UTC (permalink / raw)
To: Fangrui Song; +Cc: libc-alpha
* Fangrui Song:
>>How does lld place sections without a linker script? Purely based on
>>names?
>
> It entirely uses code to describe the built-in rules like gold, while
> GNU ld uses an internal linker script plus built-in code.
> (some rules cannot be described by the linker script language and needs
> code anyway).
How can we make sure that certain sections are covered by RELRO, and
still get start/stop symbols for them?
>>> Besides, the position of .hash in --with-default-link=no seems weird.
>>> 71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
>>> of the RO segment.
>>
>>I think it's odd that we have an old-style hash table at all. Do we
>>really need it? All tools should have migrated by now.
>
> We can remove .hash for non-mips architectures.
We need the support in glibc for most architectures in the dynamic
linker, but we don't have to build glibc with the legacy hash tables.
> (I am a bit sad .MIPS.xhash support went in, but thankfully it doesn't take
> too much code. The translation table could be solved in a better way. I
> suspect the whole thing is unnecessary with RELR.)
Huh? Aren't they unrelated?
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 7:09 ` Florian Weimer
@ 2022-04-29 7:43 ` Fangrui Song
2022-04-29 14:55 ` Florian Weimer
0 siblings, 1 reply; 26+ messages in thread
From: Fangrui Song @ 2022-04-29 7:43 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 2022-04-29, Florian Weimer wrote:
>* Fangrui Song:
>
>>>How does lld place sections without a linker script? Purely based on
>>>names?
>>
>> It entirely uses code to describe the built-in rules like gold, while
>> GNU ld uses an internal linker script plus built-in code.
>> (some rules cannot be described by the linker script language and needs
>> code anyway).
>
>How can we make sure that certain sections are covered by RELRO, and
>still get start/stop symbols for them?
About __start_/__stop_:
__start_/__stop_ symbols are special. They don't need to be mentioned in a
linker script to take effects for lld and modern GNU ld. I haven't checked
whether there is an ancient GNU ld which requires symbol assignments when the
output section description is specified.
About RELRO:
It seems that there is no section name convention to make a section
RELRO. Having a SECTIONS command with DATA_SEGMENT_ALIGN /
DATA_SEGMENT_RELRO_END can specify all RELRO sections. I do not know a
simpler approach than postprocessing ld.bfd --verbose output.
To make GNU ld work, the following linker script is sufficient:
SECTIONS {
__libc_subfreeres : { *(__libc_subfreeres) }
__libc_atexit : { *(__libc_atexit) }
__libc_IO_vtables : { *(__libc_IO_vtables) }
} INSERT BEFORE .data.rel.ro;
This script does not work with ld.lld right now because there is an
`error: section ... is not contiguous with other relro sections` from
https://reviews.llvm.org/D40359 . I need to think whether the diagnostic
can be suppressed in some cases.
>>>> Besides, the position of .hash in --with-default-link=no seems weird.
>>>> 71213dc2589554dc8f8061e9b04e80c55d098b6a (2006-09) moved it to the end
>>>> of the RO segment.
>>>
>>>I think it's odd that we have an old-style hash table at all. Do we
>>>really need it? All tools should have migrated by now.
>>
>> We can remove .hash for non-mips architectures.
>
>We need the support in glibc for most architectures in the dynamic
>linker, but we don't have to build glibc with the legacy hash tables.
Agree
>> (I am a bit sad .MIPS.xhash support went in, but thankfully it doesn't take
>> too much code. The translation table could be solved in a better way. I
>> suspect the whole thing is unnecessary with RELR.)
>
>Huh? Aren't they unrelated?
>
`DT_MIPS_LOCAL_GOTNO` and `DT_MIPS_SYMTABNO-DT_MIPS_GOTSYM` can be
defined in a way that there is no special handling. Then there will be no
requirement on the .dynsym ordering. Then the regular DT_GNU_HASH can be
used, and DT_MIPS_XHASH will be unneeded.
`DT_MIPS_LOCAL_GOTNO` optimizes on top of relative relocations but with RELR
the additional benefit is very low.
>Thanks,
>Florian
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-11 15:32 ` [PATCH v3 2/2] Default to --with-default-link=no (bug 25812) Florian Weimer
2022-04-22 6:12 ` Siddhesh Poyarekar
2022-04-28 7:25 ` Fangrui Song
@ 2022-04-29 12:59 ` Adhemerval Zanella
2022-04-29 13:17 ` Florian Weimer
2 siblings, 1 reply; 26+ messages in thread
From: Adhemerval Zanella @ 2022-04-29 12:59 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 11/04/2022 12:32, Florian Weimer via Libc-alpha wrote:
> This is necessary to place the libio vtables into the RELRO segment.
> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
> verify that this is what actually happens.
>
> The new tests fail on ia64 due to lack of (default) RELRO support
> inbutils, so they are XFAILed there.
I always frown upon new configure options, since they tend to degrade over
time (as --disable-shared for instance [1]) and require additional
maintaining costs. Why can't we make the required semantic the default
(since usually relro has minimal costs it a net gain in hardening)?
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=20845
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 12:59 ` Adhemerval Zanella
@ 2022-04-29 13:17 ` Florian Weimer
2022-04-29 13:55 ` Adhemerval Zanella
0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2022-04-29 13:17 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
* Adhemerval Zanella:
> On 11/04/2022 12:32, Florian Weimer via Libc-alpha wrote:
>> This is necessary to place the libio vtables into the RELRO segment.
>> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
>> verify that this is what actually happens.
>>
>> The new tests fail on ia64 due to lack of (default) RELRO support
>> inbutils, so they are XFAILed there.
>
> I always frown upon new configure options, since they tend to degrade over
> time (as --disable-shared for instance [1]) and require additional
> maintaining costs. Why can't we make the required semantic the default
> (since usually relro has minimal costs it a net gain in hardening)?
This is what the patch does, I think.
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 13:17 ` Florian Weimer
@ 2022-04-29 13:55 ` Adhemerval Zanella
2022-04-29 14:03 ` Florian Weimer
0 siblings, 1 reply; 26+ messages in thread
From: Adhemerval Zanella @ 2022-04-29 13:55 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 29/04/2022 10:17, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> On 11/04/2022 12:32, Florian Weimer via Libc-alpha wrote:
>>> This is necessary to place the libio vtables into the RELRO segment.
>>> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
>>> verify that this is what actually happens.
>>>
>>> The new tests fail on ia64 due to lack of (default) RELRO support
>>> inbutils, so they are XFAILed there.
>>
>> I always frown upon new configure options, since they tend to degrade over
>> time (as --disable-shared for instance [1]) and require additional
>> maintaining costs. Why can't we make the required semantic the default
>> (since usually relro has minimal costs it a net gain in hardening)?
>
> This is what the patch does, I think.
I meant to not add --with-default-link= and just not use the extra linker
script.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 13:55 ` Adhemerval Zanella
@ 2022-04-29 14:03 ` Florian Weimer
0 siblings, 0 replies; 26+ messages in thread
From: Florian Weimer @ 2022-04-29 14:03 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
* Adhemerval Zanella:
> On 29/04/2022 10:17, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> On 11/04/2022 12:32, Florian Weimer via Libc-alpha wrote:
>>>> This is necessary to place the libio vtables into the RELRO segment.
>>>> New tests elf/tst-relro-ldso and elf/tst-relro-libc are added to
>>>> verify that this is what actually happens.
>>>>
>>>> The new tests fail on ia64 due to lack of (default) RELRO support
>>>> inbutils, so they are XFAILed there.
>>>
>>> I always frown upon new configure options, since they tend to degrade over
>>> time (as --disable-shared for instance [1]) and require additional
>>> maintaining costs. Why can't we make the required semantic the default
>>> (since usually relro has minimal costs it a net gain in hardening)?
>>
>> This is what the patch does, I think.
>
> I meant to not add --with-default-link= and just not use the extra linker
> script.
That would be nice. I want to get rid of the pointer arrays, and after
the vfprintf changes (and similar vscanf changes) are merged, we won't
need the vtable array anymore, either. That should eliminate all
special section placement requirements.
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 7:43 ` Fangrui Song
@ 2022-04-29 14:55 ` Florian Weimer
2022-04-29 19:26 ` Fangrui Song
0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2022-04-29 14:55 UTC (permalink / raw)
To: Fangrui Song; +Cc: libc-alpha
* Fangrui Song:
> On 2022-04-29, Florian Weimer wrote:
>>* Fangrui Song:
>>
>>>>How does lld place sections without a linker script? Purely based on
>>>>names?
>>>
>>> It entirely uses code to describe the built-in rules like gold, while
>>> GNU ld uses an internal linker script plus built-in code.
>>> (some rules cannot be described by the linker script language and needs
>>> code anyway).
>>
>>How can we make sure that certain sections are covered by RELRO, and
>>still get start/stop symbols for them?
>
> About __start_/__stop_:
> __start_/__stop_ symbols are special. They don't need to be mentioned in a
> linker script to take effects for lld and modern GNU ld. I haven't checked
> whether there is an ancient GNU ld which requires symbol assignments when the
> output section description is specified.
According to the BFD ld documentation, __start_/__stop_ symbols are only
generated if the resulting name is a C identifier and the name is the
same. That means that if we name the section .data.rel.ro.vtables or
something like that (to benefit from .data.rel.ro.* placement), we won't
get the start/stop symbols. If we use a C identifier name for the
section, it won't be covered by RELRO.
> About RELRO:
> It seems that there is no section name convention to make a section
> RELRO. Having a SECTIONS command with DATA_SEGMENT_ALIGN /
> DATA_SEGMENT_RELRO_END can specify all RELRO sections. I do not know a
> simpler approach than postprocessing ld.bfd --verbose output.
>
> To make GNU ld work, the following linker script is sufficient:
>
> SECTIONS {
> __libc_subfreeres : { *(__libc_subfreeres) }
> __libc_atexit : { *(__libc_atexit) }
> __libc_IO_vtables : { *(__libc_IO_vtables) }
> } INSERT BEFORE .data.rel.ro;
That might work. But if we cut down the number of vtables, I think we
can eliminate those special RELRO sections altogether.
We have been slowly migrating off __libc_subfreeres and the like because
having shutdown order depend on link is very non-obvious and
problematic. For example, for optimimum results, we want to shut down
malloc last on a thread.
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-22 6:35 ` Florian Weimer
2022-04-22 8:12 ` Siddhesh Poyarekar
2022-04-22 8:56 ` Andreas Schwab
@ 2022-04-29 16:02 ` Dmitry V. Levin
2022-04-29 16:14 ` Florian Weimer
2 siblings, 1 reply; 26+ messages in thread
From: Dmitry V. Levin @ 2022-04-29 16:02 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, libc-alpha
On Fri, Apr 22, 2022 at 08:35:37AM +0200, Florian Weimer via Libc-alpha wrote:
> * Siddhesh Poyarekar:
>
> >> diff --git a/INSTALL b/INSTALL
> >> index 63c022d6b9..de150f66eb 100644
> >> --- a/INSTALL
> >> +++ b/INSTALL
> >> @@ -90,6 +90,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
> >> library will still be usable, but functionality may be lost--for
> >> example, you can't build a shared libc with old binutils.
> >> +'--with-default-link=FLAG'
> >> + With '--with-default-link=yes', the GNU C Library does not use a
> >> + custom linker scipt for linking its individual shared objects. The
> >> + default for FLAG is the opposite, 'no', because the custom linker
> >> + script is needed for full RELRO protection.
> >> +
> >
> > Andreas' comments still apply here I think, i.e. fix the "scipt" type
> > and rephrase so that it's clearer that the option controls the library
> > build process and not the library itself.
>
> I thought I had fixed this. What about this?
>
> '--with-default-link=FLAG'
> With '--with-default-link=yes', the build system does not use a
> custom linker script for linking shared objects. The default for
> FLAG is the opposite, 'no', because the custom linker script is
> needed for full RELRO protection.
>
> > Does it make sense to make this --disable-custom-link or
> > --enable-default-link instead, since the option is binary? The --with
> > prefix is typically for richer options, e.g. paths. Suggest something
> > like this:
> >
> > --disable-custom-link
> > Don't use a custom linker script to build the GNU C Library,
> > preferring the static linker's default script instead. The custom
> > linker script is needed for full RELRO protection.
>
> I want to backport this, and distributions are already using this
> option, so I prefer not to make changes here.
I'm sorry to remind you something you know pretty well - the traditional
recommendations on choosing --enable-FEATURE[=ARG]/--disable-FEATURE[1]
vs --with-PACKAGE[=ARG]/--without-PACKAGE[2].
The first pair of options is to specify which build-time features to
enable, and the second one is to specify which external software to use.
If a downstream does not follow the documentation, this fact shouldn't be
normally used as a rationale for not following the documentation upstream.
If changes are accepted upstream as is just because they are already in
use downstream, this would open the way to all kinds of sloppy changes.
[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.71/html_node/Package-Options.html
[2] https://www.gnu.org/software/autoconf/manual/autoconf-2.71/html_node/External-Software.html
--
ldv
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 16:02 ` Dmitry V. Levin
@ 2022-04-29 16:14 ` Florian Weimer
2022-04-29 20:48 ` Dmitry V. Levin
0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2022-04-29 16:14 UTC (permalink / raw)
To: Dmitry V. Levin; +Cc: Siddhesh Poyarekar, libc-alpha
* Dmitry V. Levin:
> On Fri, Apr 22, 2022 at 08:35:37AM +0200, Florian Weimer via Libc-alpha wrote:
>> * Siddhesh Poyarekar:
>>
>> >> diff --git a/INSTALL b/INSTALL
>> >> index 63c022d6b9..de150f66eb 100644
>> >> --- a/INSTALL
>> >> +++ b/INSTALL
>> >> @@ -90,6 +90,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
>> >> library will still be usable, but functionality may be lost--for
>> >> example, you can't build a shared libc with old binutils.
>> >> +'--with-default-link=FLAG'
>> >> + With '--with-default-link=yes', the GNU C Library does not use a
>> >> + custom linker scipt for linking its individual shared objects. The
>> >> + default for FLAG is the opposite, 'no', because the custom linker
>> >> + script is needed for full RELRO protection.
>> >> +
>> >
>> > Andreas' comments still apply here I think, i.e. fix the "scipt" type
>> > and rephrase so that it's clearer that the option controls the library
>> > build process and not the library itself.
>>
>> I thought I had fixed this. What about this?
>>
>> '--with-default-link=FLAG'
>> With '--with-default-link=yes', the build system does not use a
>> custom linker script for linking shared objects. The default for
>> FLAG is the opposite, 'no', because the custom linker script is
>> needed for full RELRO protection.
>>
>> > Does it make sense to make this --disable-custom-link or
>> > --enable-default-link instead, since the option is binary? The --with
>> > prefix is typically for richer options, e.g. paths. Suggest something
>> > like this:
>> >
>> > --disable-custom-link
>> > Don't use a custom linker script to build the GNU C Library,
>> > preferring the static linker's default script instead. The custom
>> > linker script is needed for full RELRO protection.
>>
>> I want to backport this, and distributions are already using this
>> option, so I prefer not to make changes here.
>
> I'm sorry to remind you something you know pretty well - the traditional
> recommendations on choosing --enable-FEATURE[=ARG]/--disable-FEATURE[1]
> vs --with-PACKAGE[=ARG]/--without-PACKAGE[2].
>
> The first pair of options is to specify which build-time features to
> enable, and the second one is to specify which external software to use.
>
> If a downstream does not follow the documentation, this fact shouldn't be
> normally used as a rationale for not following the documentation upstream.
>
> If changes are accepted upstream as is just because they are already in
> use downstream, this would open the way to all kinds of sloppy changes.
I think your comment is based on an assumption that this is a new
option?
This change did not add the --with-default-link configure option, only
the documentation for it, and it changed the default to the safe one.
Thanks,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 14:55 ` Florian Weimer
@ 2022-04-29 19:26 ` Fangrui Song
0 siblings, 0 replies; 26+ messages in thread
From: Fangrui Song @ 2022-04-29 19:26 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 2022-04-29, Florian Weimer wrote:
>* Fangrui Song:
>
>> On 2022-04-29, Florian Weimer wrote:
>>>* Fangrui Song:
>>>
>>>>>How does lld place sections without a linker script? Purely based on
>>>>>names?
>>>>
>>>> It entirely uses code to describe the built-in rules like gold, while
>>>> GNU ld uses an internal linker script plus built-in code.
>>>> (some rules cannot be described by the linker script language and needs
>>>> code anyway).
>>>
>>>How can we make sure that certain sections are covered by RELRO, and
>>>still get start/stop symbols for them?
>>
>> About __start_/__stop_:
>> __start_/__stop_ symbols are special. They don't need to be mentioned in a
>> linker script to take effects for lld and modern GNU ld. I haven't checked
>> whether there is an ancient GNU ld which requires symbol assignments when the
>> output section description is specified.
>
>According to the BFD ld documentation, __start_/__stop_ symbols are only
>generated if the resulting name is a C identifier and the name is the
>same. That means that if we name the section .data.rel.ro.vtables or
>something like that (to benefit from .data.rel.ro.* placement), we won't
>get the start/stop symbols. If we use a C identifier name for the
>section, it won't be covered by RELRO.
I know the rule. I meant I don't find a way making lld build have
RELRO __libc_* without the help of ld.bfd --verbose ;-)
>> About RELRO:
>> It seems that there is no section name convention to make a section
>> RELRO. Having a SECTIONS command with DATA_SEGMENT_ALIGN /
>> DATA_SEGMENT_RELRO_END can specify all RELRO sections. I do not know a
>> simpler approach than postprocessing ld.bfd --verbose output.
>>
>> To make GNU ld work, the following linker script is sufficient:
>>
>> SECTIONS {
>> __libc_subfreeres : { *(__libc_subfreeres) }
>> __libc_atexit : { *(__libc_atexit) }
>> __libc_IO_vtables : { *(__libc_IO_vtables) }
>> } INSERT BEFORE .data.rel.ro;
>
>That might work. But if we cut down the number of vtables, I think we
>can eliminate those special RELRO sections altogether.
>
>We have been slowly migrating off __libc_subfreeres and the like because
>having shutdown order depend on link is very non-obvious and
>problematic. For example, for optimimum results, we want to shut down
>malloc last on a thread.
If there is a way to get rid of __libc_* output sections, it will
certainly be great!
>Thanks,
>Florian
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 2/2] Default to --with-default-link=no (bug 25812)
2022-04-29 16:14 ` Florian Weimer
@ 2022-04-29 20:48 ` Dmitry V. Levin
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry V. Levin @ 2022-04-29 20:48 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, libc-alpha
On Fri, Apr 29, 2022 at 06:14:27PM +0200, Florian Weimer wrote:
> * Dmitry V. Levin:
>
> > On Fri, Apr 22, 2022 at 08:35:37AM +0200, Florian Weimer via Libc-alpha wrote:
> >> * Siddhesh Poyarekar:
> >>
> >> >> diff --git a/INSTALL b/INSTALL
> >> >> index 63c022d6b9..de150f66eb 100644
> >> >> --- a/INSTALL
> >> >> +++ b/INSTALL
> >> >> @@ -90,6 +90,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
> >> >> library will still be usable, but functionality may be lost--for
> >> >> example, you can't build a shared libc with old binutils.
> >> >> +'--with-default-link=FLAG'
> >> >> + With '--with-default-link=yes', the GNU C Library does not use a
> >> >> + custom linker scipt for linking its individual shared objects. The
> >> >> + default for FLAG is the opposite, 'no', because the custom linker
> >> >> + script is needed for full RELRO protection.
> >> >> +
> >> >
> >> > Andreas' comments still apply here I think, i.e. fix the "scipt" type
> >> > and rephrase so that it's clearer that the option controls the library
> >> > build process and not the library itself.
> >>
> >> I thought I had fixed this. What about this?
> >>
> >> '--with-default-link=FLAG'
> >> With '--with-default-link=yes', the build system does not use a
> >> custom linker script for linking shared objects. The default for
> >> FLAG is the opposite, 'no', because the custom linker script is
> >> needed for full RELRO protection.
> >>
> >> > Does it make sense to make this --disable-custom-link or
> >> > --enable-default-link instead, since the option is binary? The --with
> >> > prefix is typically for richer options, e.g. paths. Suggest something
> >> > like this:
> >> >
> >> > --disable-custom-link
> >> > Don't use a custom linker script to build the GNU C Library,
> >> > preferring the static linker's default script instead. The custom
> >> > linker script is needed for full RELRO protection.
> >>
> >> I want to backport this, and distributions are already using this
> >> option, so I prefer not to make changes here.
> >
> > I'm sorry to remind you something you know pretty well - the traditional
> > recommendations on choosing --enable-FEATURE[=ARG]/--disable-FEATURE[1]
> > vs --with-PACKAGE[=ARG]/--without-PACKAGE[2].
> >
> > The first pair of options is to specify which build-time features to
> > enable, and the second one is to specify which external software to use.
> >
> > If a downstream does not follow the documentation, this fact shouldn't be
> > normally used as a rationale for not following the documentation upstream.
> >
> > If changes are accepted upstream as is just because they are already in
> > use downstream, this would open the way to all kinds of sloppy changes.
>
> I think your comment is based on an assumption that this is a new
> option?
Yes, the suggestion of new option --disable-custom-link somehow made me
think that --with-default-link is also a new option name, not something
that goes back to 2011.
--
ldv
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2022-04-29 20:48 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 15:32 [PATCH v3 1/2] scripts: Add glibcelf.py module Florian Weimer
2022-04-11 15:32 ` [PATCH v3 2/2] Default to --with-default-link=no (bug 25812) Florian Weimer
2022-04-22 6:12 ` Siddhesh Poyarekar
2022-04-22 6:35 ` Florian Weimer
2022-04-22 8:12 ` Siddhesh Poyarekar
2022-04-22 8:24 ` Florian Weimer
2022-04-22 8:32 ` Siddhesh Poyarekar
2022-04-22 8:34 ` Florian Weimer
2022-04-22 8:36 ` Siddhesh Poyarekar
2022-04-22 8:56 ` Andreas Schwab
2022-04-29 16:02 ` Dmitry V. Levin
2022-04-29 16:14 ` Florian Weimer
2022-04-29 20:48 ` Dmitry V. Levin
2022-04-28 7:25 ` Fangrui Song
2022-04-28 8:40 ` Florian Weimer
2022-04-29 6:00 ` Fangrui Song
2022-04-29 7:09 ` Florian Weimer
2022-04-29 7:43 ` Fangrui Song
2022-04-29 14:55 ` Florian Weimer
2022-04-29 19:26 ` Fangrui Song
2022-04-29 12:59 ` Adhemerval Zanella
2022-04-29 13:17 ` Florian Weimer
2022-04-29 13:55 ` Adhemerval Zanella
2022-04-29 14:03 ` Florian Weimer
2022-04-21 15:54 ` [PATCH v3 1/2] scripts: Add glibcelf.py module Siddhesh Poyarekar
2022-04-21 20:23 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).