The ELF loader in imgact_elf.c contains the following code to verify a program header before loading it: /* * It's necessary to fail if the filsz + offset taken from the * header is greater than the actual file pager object's size. * If we were to allow this, then the vm_map_find() below would * walk right off the end of the file object and into the ether. * * While I'm here, might as well check for something else that * is invalid: filsz cannot be greater than memsz. */ if ((off_t)filsz + offset > imgp->attr->va_size || filsz > memsz) { uprintf("elf_load_section: truncated ELF file\n"); return (ENOEXEC); } However, this code is incorrect. If a program header corresponds to sections that are all marked NOBITS, GNU ld generates a program header with p_filesz == 0 and p_offset at the next aligned offset just past the end of the file. This is fine as no bytes are actually ever read from the binary. However, FreeBSD refuses to load such a valid ELF binary. I request to amend this verification procedure to add a special case for program headers with p_filesz == 0: if (filesz > memsz || filesz > 0 && (off_t)filesz + offset > imgp->attr->va_size)
As an example, save the following file in test.S and type cc -c test.S ld -o test test.o ./test on an amd64 machine to reproduce the problem. test.S: #include <sys/syscall.h> .bss .align 4096 x: .space 16 .text .globl _start _start: mov $SYS_exit,%eax xor %edi,%edi syscall
So essentially, objects with no non-zero static data can't be loaded? Is this an issue in practice? I'd guess crt0 adds some initialized data, so most programs will not run into this?
You are correct in both observations. Programs linked against crt0.o should never trigger this bug, which is probably how it went undiscovered for so long. If there is interest, I can write and test a patch. This issue can affect programs that do not link against the libc and that do not use initialized data either.
Created attachment 180607 [details] Handle filsz == 0. Attached is the proposed patch. Note that it is somewhat more involved that just passing the filsz == 0 in the initial ENOEXEC check, because I do not want even think what happens when offset is not multiple of the page size. I just force any file-backed COW mappings to be avoided if filesz == 0. Also the patch contains some style cleanup, which I split for actual commit.
Created attachment 180608 [details] Handle filsz == 0. Correctly handle the memsz/filsz staircase (forgot else).
A commit references this bug: Author: kib Date: Sun Mar 12 13:51:13 UTC 2017 New revision: 315157 URL: https://svnweb.freebsd.org/changeset/base/315157 Log: Accept linkers representation for ELF segments with zero on-disk length. For such segments, GNU bfd linker writes knowingly incorrect value into the the file offset field of the program header entry, with the motivation that file should not be mapped for creation of this segment at all. Relax checks for the ELF structure validity when on-disk segment length is zero, and explicitely set mapping length to zero for such segments to avoid validating rounding arithmetic. PR: 217610 Reported by: Robert Clausecker <fuz@fuz.su> Sponsored by: The FreeBSD Foundation MFC after: 1 week Changes: head/sys/kern/imgact_elf.c
For reference ld.lld creates an 8928-byte file for your sample, with the following segments: There are 5 program headers, starting at offset 64 Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flags Align PHDR 0x0000000000000040 0x0000000000200040 0x0000000000200040 0x0000000000000118 0x0000000000000118 R 8 LOAD 0x0000000000000000 0x0000000000200000 0x0000000000200000 0x0000000000000158 0x0000000000000158 R 1000 LOAD 0x0000000000001000 0x0000000000201000 0x0000000000201000 0x0000000000000009 0x0000000000000009 R E 1000 LOAD 0x0000000000002000 0x0000000000202000 0x0000000000202000 0x0000000000000000 0x0000000000000010 RW 1000 GNU_STACK 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 RW 0