From b31281a5b6f82166e601ebd49d00aac59dedc9a6 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Wed, 15 Jun 2022 18:27:49 +0200 Subject: [PATCH] runtime: scan all writable program segments Previously we used to scan between _edata and _end. This is not correct: the .data section starts *before* _edata. Fixing this would mean changing _edata to _etext, but that didn't quite work either. It appears that there are inaccessible pages between _etext and _end on ARM. Therefore, a different solution was needed. What I've implemented is similar to Windows and MacOS: namely, finding writable segments by parsing the program header of the currently running program. It's a lot more verbose, but it should be correct on all architectures. It probably also reduces the globals to scan to those that _really_ need to be scanned. This bug didn't result in issues in CI, but did result in a bug in the recover branch: https://github.com/tinygo-org/tinygo/pull/2331. This patch fixes this bug. --- src/runtime/os_linux.go | 95 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 84 insertions(+), 11 deletions(-) diff --git a/src/runtime/os_linux.go b/src/runtime/os_linux.go index 2e42515a..8f157265 100644 --- a/src/runtime/os_linux.go +++ b/src/runtime/os_linux.go @@ -24,21 +24,94 @@ const ( clock_MONOTONIC_RAW = 4 ) -//go:extern _edata -var globalsStartSymbol [0]byte +// For the definition of the various header structs, see: +// https://refspecs.linuxfoundation.org/elf/elf.pdf +// Also useful: +// https://en.wikipedia.org/wiki/Executable_and_Linkable_Format +type elfHeader struct { + ident_magic uint32 + ident_class uint8 + ident_data uint8 + ident_version uint8 + ident_osabi uint8 + ident_abiversion uint8 + _ [7]byte // reserved + filetype uint16 + machine uint16 + version uint32 + entry uintptr + phoff uintptr + shoff uintptr + flags uint32 + ehsize uint16 + phentsize uint16 + phnum uint16 + shentsize uint16 + shnum uint16 + shstrndx uint16 +} -//go:extern _end -var globalsEndSymbol [0]byte +type elfProgramHeader64 struct { + _type uint32 + flags uint32 + offset uintptr + vaddr uintptr + paddr uintptr + filesz uintptr + memsz uintptr + align uintptr +} + +type elfProgramHeader32 struct { + _type uint32 + offset uintptr + vaddr uintptr + paddr uintptr + filesz uintptr + memsz uintptr + flags uint32 + align uintptr +} + +// ELF header of the currently running process. +// +//go:extern __ehdr_start +var ehdr_start elfHeader // markGlobals marks all globals, which are reachable by definition. -// -// This implementation marks all globals conservatively and assumes it can use -// linker-defined symbols for the start and end of the .data section. +// It parses the ELF program header to find writable segments. func markGlobals() { - start := uintptr(unsafe.Pointer(&globalsStartSymbol)) - end := uintptr(unsafe.Pointer(&globalsEndSymbol)) - start = (start + unsafe.Alignof(uintptr(0)) - 1) &^ (unsafe.Alignof(uintptr(0)) - 1) // align on word boundary - markRoots(start, end) + // Relevant constants from the ELF specification. + // See: https://refspecs.linuxfoundation.org/elf/elf.pdf + const ( + PT_LOAD = 1 + PF_W = 0x2 // program flag: write access + ) + + headerPtr := unsafe.Pointer(uintptr(unsafe.Pointer(&ehdr_start)) + ehdr_start.phoff) + for i := 0; i < int(ehdr_start.phnum); i++ { + // Look for a writable segment and scan its contents. + // There is a little bit of duplication here, which is unfortunate. But + // the alternative would be to put elfProgramHeader in separate files + // which is IMHO a lot uglier. If only the ELF spec was consistent + // between 32-bit and 64-bit... + if TargetBits == 64 { + header := (*elfProgramHeader64)(headerPtr) + if header._type == PT_LOAD && header.flags&PF_W != 0 { + start := header.vaddr + end := start + header.memsz + markRoots(start, end) + } + } else { + header := (*elfProgramHeader32)(headerPtr) + if header._type == PT_LOAD && header.flags&PF_W != 0 { + start := header.vaddr + end := start + header.memsz + markRoots(start, end) + } + } + headerPtr = unsafe.Pointer(uintptr(headerPtr) + uintptr(ehdr_start.phentsize)) + } } //export getpagesize