Validate Overlapping Memory Regions in ELF Loader #150
Conversation
| vaddr: text_section.sh_addr.saturating_add(ebpf::MM_PROGRAM_START), | ||
| offset_range: text_section.file_range(), | ||
| }; | ||
| if text_section_info.vaddr > ebpf::MM_STACK_START { |
There was a problem hiding this comment.
the overlap checks elsewhere won't catch this case?
There was a problem hiding this comment.
This checks the vaddr, other places check the file offset. Or is there a specific one that does that too?
src/elf.rs
Outdated
| .collect::<Vec<_>>(); | ||
| ro_section_infos.sort_by(|a, b| a.vaddr.cmp(&b.vaddr)); | ||
| for i in 0..ro_section_infos.len() { | ||
| if ro_section_infos[i].vaddr > ebpf::MM_STACK_START { |
There was a problem hiding this comment.
Would a final check of all regions (or on each region add) for overlapping be more robust? I like that this is focused on ELF validation but I wonder if we should have a higher level elf rejection here where any overlapping elf section should cause a failure. And, we are probably going to start adding new sections at runtime related to ephemeral accounts or whatever else we plan to map into the program space. Maybe doing this check in the memory_mappings would help solve both?
There was a problem hiding this comment.
Yes, I also considered that, was just a bit more involved due to introducing a Result type in the constructor of MemoryMapping. Anyway, I moved the overlap checks there.
Lichtso
force-pushed
the
fix/overlapping_memory_regions
branch
from
cf876c7 to
b132834
Compare
SectionInfostruct for error messagesVirtualAddressOverlapif adjacentMemorRegions overlap in virtual address spaceOutOfBoundsif sections fromMM_PROGRAM_STARTreach intoMM_STACK_START