diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 8ee398aa8d..f1cda814f3 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -24,13 +24,13 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include #include #include #include #include #include #include -#include #include #include #include @@ -301,6 +301,13 @@ static bool validate_inode_mmap_prot(const Process& process, int prot, const Ino return false; if ((prot & PROT_READ) && !metadata.may_read(process)) return false; + InterruptDisabler disabler; + if (inode.vmobject()) { + if ((prot & PROT_EXEC) && inode.vmobject()->writable_mappings()) + return false; + if ((prot & PROT_WRITE) && inode.vmobject()->executable_mappings()) + return false; + } return true; } @@ -708,6 +715,18 @@ int Process::do_exec(NonnullRefPtr main_program_description, Ve if (parts.is_empty()) return -ENOENT; + RefPtr vmobject; + if (interpreter_description) { + vmobject = InodeVMObject::create_with_inode(*interpreter_description->inode()); + } else { + vmobject = InodeVMObject::create_with_inode(*main_program_description->inode()); + } + + if (static_cast(*vmobject).writable_mappings()) { + dbg() << "Refusing to execute a write-mapped program"; + return -ETXTBSY; + } + u32 entry_eip = 0; // FIXME: Is there a race here? auto old_page_directory = move(m_page_directory); @@ -717,10 +736,7 @@ int Process::do_exec(NonnullRefPtr main_program_description, Ve #endif ProcessPagingScope paging_scope(*this); - struct VMObjectAndRegion { - NonnullRefPtr vmobject; - Region* region; - }; + Region* region { nullptr }; InodeMetadata loader_metadata; @@ -731,22 +747,16 @@ int Process::do_exec(NonnullRefPtr main_program_description, Ve // 0x08000000 is a verified random number chosen by random dice roll https://xkcd.com/221/ u32 totally_random_offset = interpreter_description ? 0x08000000 : 0; - auto [vmobject, region] = [&]() { - // FIXME: We should be able to load both the PT_INTERP interpreter and the main program... once the RTLD is smart enough - if (interpreter_description) { - loader_metadata = interpreter_description->metadata(); - auto vmobject = InodeVMObject::create_with_inode(*interpreter_description->inode()); - auto region = allocate_region_with_vmobject(VirtualAddress(), loader_metadata.size, vmobject, 0, interpreter_description->absolute_path(), PROT_READ, false); - // we don't need the interpreter file desciption after we've loaded (or not) it into memory - interpreter_description = nullptr; - return VMObjectAndRegion { move(vmobject), region }; - } - + // FIXME: We should be able to load both the PT_INTERP interpreter and the main program... once the RTLD is smart enough + if (interpreter_description) { + loader_metadata = interpreter_description->metadata(); + region = allocate_region_with_vmobject(VirtualAddress(), loader_metadata.size, *vmobject, 0, interpreter_description->absolute_path(), PROT_READ, false); + // we don't need the interpreter file desciption after we've loaded (or not) it into memory + interpreter_description = nullptr; + } else { loader_metadata = main_program_description->metadata(); - auto vmobject = InodeVMObject::create_with_inode(*main_program_description->inode()); - auto region = allocate_region_with_vmobject(VirtualAddress(), loader_metadata.size, vmobject, 0, main_program_description->absolute_path(), PROT_READ, false); - return VMObjectAndRegion { move(vmobject), region }; - }(); + region = allocate_region_with_vmobject(VirtualAddress(), loader_metadata.size, *vmobject, 0, main_program_description->absolute_path(), PROT_READ, false); + } ASSERT(region); @@ -789,7 +799,7 @@ int Process::do_exec(NonnullRefPtr main_program_description, Ve prot |= PROT_WRITE; if (is_executable) prot |= PROT_EXEC; - if (auto* region = allocate_region_with_vmobject(vaddr.offset(totally_random_offset), size, vmobject, offset_in_image, String(name), prot)) + if (auto* region = allocate_region_with_vmobject(vaddr.offset(totally_random_offset), size, *vmobject, offset_in_image, String(name), prot)) return region->vaddr().as_ptr(); return nullptr; }; @@ -828,7 +838,7 @@ int Process::do_exec(NonnullRefPtr main_program_description, Ve // instead of just non-null. You could totally have a DSO with entry point of // the beginning of the text segement. if (!loader->entry().offset(totally_random_offset).get()) { - kprintf("do_exec: Failure loading %s, entry pointer is invalid! (%p)", path.characters(), loader->entry().offset(totally_random_offset).get()); + kprintf("do_exec: Failure loading %s, entry pointer is invalid! (%p)\n", path.characters(), loader->entry().offset(totally_random_offset).get()); return -ENOEXEC; } diff --git a/Kernel/VM/InodeVMObject.cpp b/Kernel/VM/InodeVMObject.cpp index b994f84e0d..36a97b87bb 100644 --- a/Kernel/VM/InodeVMObject.cpp +++ b/Kernel/VM/InodeVMObject.cpp @@ -175,3 +175,23 @@ int InodeVMObject::release_all_clean_pages_impl() }); return count; } + +u32 InodeVMObject::writable_mappings() const +{ + u32 count = 0; + const_cast(*this).for_each_region([&](auto& region) { + if (region.is_writable()) + ++count; + }); + return count; +} + +u32 InodeVMObject::executable_mappings() const +{ + u32 count = 0; + const_cast(*this).for_each_region([&](auto& region) { + if (region.is_executable()) + ++count; + }); + return count; +} diff --git a/Kernel/VM/InodeVMObject.h b/Kernel/VM/InodeVMObject.h index 05273f70ca..3f5705948d 100644 --- a/Kernel/VM/InodeVMObject.h +++ b/Kernel/VM/InodeVMObject.h @@ -47,6 +47,9 @@ public: int release_all_clean_pages(); + u32 writable_mappings() const; + u32 executable_mappings() const; + private: explicit InodeVMObject(Inode&, size_t); explicit InodeVMObject(const InodeVMObject&); diff --git a/Tests/Kernel/elf-execve-mmap-race.cpp b/Tests/Kernel/elf-execve-mmap-race.cpp new file mode 100644 index 0000000000..2928a30f1e --- /dev/null +++ b/Tests/Kernel/elf-execve-mmap-race.cpp @@ -0,0 +1,128 @@ +#include +#include +#include +#include +#include +#include +#include + +volatile bool hax = false; + +int main() +{ + char buffer[16384]; + + auto& header = *(Elf32_Ehdr*)buffer; + header.e_ident[EI_MAG0] = ELFMAG0; + header.e_ident[EI_MAG1] = ELFMAG1; + header.e_ident[EI_MAG2] = ELFMAG2; + header.e_ident[EI_MAG3] = ELFMAG3; + header.e_ident[EI_CLASS] = ELFCLASS32; + header.e_ident[EI_DATA] = ELFDATA2LSB; + header.e_ident[EI_VERSION] = EV_CURRENT; + header.e_ident[EI_OSABI] = ELFOSABI_SYSV; + header.e_ident[EI_ABIVERSION] = 0; + header.e_type = ET_EXEC; + header.e_version = EV_CURRENT; + header.e_ehsize = sizeof(Elf32_Ehdr); + header.e_machine = EM_386; + header.e_shentsize = sizeof(Elf32_Shdr); + + header.e_phnum = 1; + header.e_phoff = 52; + header.e_phentsize = sizeof(Elf32_Phdr); + + auto* ph = (Elf32_Phdr*)(&buffer[header.e_phoff]); + ph[0].p_vaddr = 0x20000000; + ph[0].p_type = PT_LOAD; + ph[0].p_filesz = sizeof(buffer); + ph[0].p_memsz = sizeof(buffer); + ph[0].p_flags = PF_R | PF_W; + ph[0].p_align = PAGE_SIZE; + + header.e_shnum = 3; + header.e_shoff = 1024; + + u32 secret_address = 0x00184658; + + auto* sh = (Elf32_Shdr*)(&buffer[header.e_shoff]); + sh[0].sh_type = SHT_SYMTAB; + sh[0].sh_offset = 2048; + sh[0].sh_entsize = sizeof(Elf32_Sym); + sh[0].sh_size = 1 * sizeof(Elf32_Sym); + + sh[1].sh_type = SHT_STRTAB; + sh[1].sh_offset = secret_address - 0x01001000; + sh[1].sh_entsize = 0; + sh[1].sh_size = 1024; + + sh[2].sh_type = SHT_STRTAB; + sh[2].sh_offset = 4096; + sh[2].sh_entsize = 0; + sh[2].sh_size = 1024; + header.e_shstrndx = 2; + + auto* sym = (Elf32_Sym*)(&buffer[2048]); + sym[0].st_value = 0; + sym[0].st_name = 0; + + header.e_entry = 0; + + int fd = open("x", O_RDWR | O_CREAT, 0777); + if (fd < 0) { + perror("open"); + return 1; + } + + int nwritten = write(fd, buffer, sizeof(buffer)); + if (nwritten < 0) { + perror("write"); + return 1; + } + + sync(); + + auto* mapped = (u8*)mmap(nullptr, sizeof(buffer), PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 0); + if (mapped == MAP_FAILED) { + perror("mmap"); + return 1; + } + + auto* writable_program_headers = (Elf32_Phdr*)(&mapped[header.e_phoff]); + + pthread_attr_t attrs; + pthread_attr_init(&attrs); + sched_param high_prio { 99 }; + pthread_attr_setschedparam(&attrs, &high_prio); + + pthread_t t; + pthread_create( + &t, &attrs, [](void* ctx) -> void* { + auto& ph = *(volatile Elf32_Phdr*)ctx; + for (;;) { + if (!hax) + ph.p_offset = 0x60000000; + else + ph.p_offset = 0; + hax = !hax; + usleep(1); + } + return nullptr; + }, + &writable_program_headers[0]); + + for (;;) { + + if (!fork()) { + try_again: + printf("exec\n"); + if (execl("/home/anon/x", "x", nullptr) < 0) { + } + goto try_again; + } + + printf("waitpid\n"); + waitpid(-1, nullptr, 0); + } + return 0; +}