On Mon, Nov 7, 2022 at 3:59 AM Kees Cook keescook@chromium.org wrote:
Thanks for the patch! I need to triple-check this logic, as there have been some overlapping (or out-of-order) LOAD bugs in the past too, and I want to make sure we don't accidentally zero things that already got loaded, etc.
Hi Kees,
Thanks for looking at my patch. I've submitted an (unprompted) v2 with an additional fix for a loading bug that could load segments after a .bss on top of .bss itself, which would overwrite any bss zeroing efforts. Note that this bug was already present in load_elf_binary.
See a repro at https://github.com/heatd/elf-bug-questionmark, and the comments on the patch/repro for more info. Most ELF loading operating systems out there seem to fail on this one.
As for overlapping/out-of-order LOAD segments, what kind of handling do we want here? The ELF spec (http://www.sco.com/developers/gabi/latest/ch5.pheader.html) says that "Loadable segment entries in the program header table appear in ascending order, sorted on the p_vaddr member.", so do we really want to support that? My -v2 was substantially simplified based on assuming ELF-compliant executables.
David, has there been any work on adding a way to instantiate userspace VMAs in a KUnit test? I tried to write this myself, but I couldn't figure out how to make the userspace memory mappings appear. Here's my fumbling attempt: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=devel/...
I really wish KUnit had userspace mapping support -- I have a bunch of unit tests that need to get built up around checking for regressions here, etc.
+1 on getting this unit-tested, this is a bit of a minefield
Thanks, Pedro