Currently, the ELF image activator only works with PT_INTERP and PT_NOTE fully contained in the first page of the executable. This is unneccessary restrictive, and more, requires special hacks in the linker. It is not too hard to eliminate the limitation by reading arbirtrary file location to get interpreter path and notes. Still, the interpreter path length is limited by MAXPATHLEN, note segment is supposed to be at most page size (typical size is several dozen of bytes).
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Also handle notes in the rtld.
The misconfigured ld may create executables where phdr is not located in the PT_LOADed region. Such image segfaults in rtld during activation; patch does not help there.
| sys/kern/imgact_elf.c | ||
|---|---|---|
| 794–795 | We need to detect either start or end of phdr falling outside of the first page. I do not see how second condition alone is enough. Note that we could get overflow if p_offset is outside the first page, and then the second condition is true. | |
| 798 | Done. | |
| 2164 | We can, sure. But as I said in the annotation, typical notes segment is around twenty bytes, we only have two or three (on arm) notes defined, with 4-byte payload. We can put almost arbitrary limit there after the patch, but some limit must be present. | |
| sys/kern/imgact_elf.c | ||
|---|---|---|
| 794–795 | You probably mean this: if (phdr[i].p_offset + interp_name_len > PAGE_SIZE) {But it still can overflow. | |
| sys/kern/imgact_elf.c | ||
|---|---|---|
| 2164 | Fair enough. On first glance it just seems like PAGE_SIZE is an inherent limit of the mechanism, not a somewhat arbitrary but convenient limit. | |
Comment Actions
Looks fine to me in general, just a few suggestions.
| libexec/rtld-elf/map_object.c | ||
|---|---|---|
| 161 | Can you use MAP_FAILED here? | |
| 199 | I see this doesn't use MAP_FAILED either (though it uses a caddr_t cast instead of (char *)), but I think it would be best to use MAP_FAILED in new changes and eventually use it in all of rtld. | |
| 320 | And here? | |
| sys/kern/imgact_elf.c | ||
| 2164 | Might be fine to note that in a comment above this line then. ("We need some limit, might as well use PAGE_SIZE.") | |
| sys/kern/imgact_elf.c | ||
|---|---|---|
| 2164 | Right - the explanation from the Phabricator summary above could go in a comment here. | |