Selaa lähdekoodia

Kernel: Fix use-after-free in sys$mremap

Now that Region::name() has been changed to return a StringView we
can't rely on keeping a copy of the region's name past the region's
destruction just by holding a copy of the StringView.
Gunnar Beutner 4 vuotta sitten
vanhempi
commit
fe0ae3161a
2 muutettua tiedostoa jossa 4 lisäystä ja 2 poistoa
  1. 3 2
      Kernel/Syscalls/mmap.cpp
  2. 1 0
      Kernel/VM/Region.h

+ 3 - 2
Kernel/Syscalls/mmap.cpp

@@ -536,7 +536,6 @@ KResultOr<FlatPtr> Process::sys$mremap(Userspace<const Syscall::SC_mremap_params
 
 
     if (old_region->vmobject().is_shared_inode() && params.flags & MAP_PRIVATE && !(params.flags & (MAP_ANONYMOUS | MAP_NORESERVE))) {
     if (old_region->vmobject().is_shared_inode() && params.flags & MAP_PRIVATE && !(params.flags & (MAP_ANONYMOUS | MAP_NORESERVE))) {
         auto range = old_region->range();
         auto range = old_region->range();
-        auto old_name = old_region->name();
         auto old_prot = region_access_flags_to_prot(old_region->access());
         auto old_prot = region_access_flags_to_prot(old_region->access());
         auto old_offset = old_region->offset_in_vmobject();
         auto old_offset = old_region->offset_in_vmobject();
         NonnullRefPtr inode = static_cast<SharedInodeVMObject&>(old_region->vmobject()).inode();
         NonnullRefPtr inode = static_cast<SharedInodeVMObject&>(old_region->vmobject()).inode();
@@ -545,12 +544,14 @@ KResultOr<FlatPtr> Process::sys$mremap(Userspace<const Syscall::SC_mremap_params
         if (!new_vmobject)
         if (!new_vmobject)
             return ENOMEM;
             return ENOMEM;
 
 
+        auto old_name = old_region->take_name();
+
         // Unmap without deallocating the VM range since we're going to reuse it.
         // Unmap without deallocating the VM range since we're going to reuse it.
         old_region->unmap(Region::ShouldDeallocateVirtualMemoryRange::No);
         old_region->unmap(Region::ShouldDeallocateVirtualMemoryRange::No);
         bool success = space().deallocate_region(*old_region);
         bool success = space().deallocate_region(*old_region);
         VERIFY(success);
         VERIFY(success);
 
 
-        auto new_region_or_error = space().allocate_region_with_vmobject(range, new_vmobject.release_nonnull(), old_offset, old_name, old_prot, false);
+        auto new_region_or_error = space().allocate_region_with_vmobject(range, new_vmobject.release_nonnull(), old_offset, old_name->view(), old_prot, false);
         if (new_region_or_error.is_error())
         if (new_region_or_error.is_error())
             return new_region_or_error.error().error();
             return new_region_or_error.error().error();
         auto& new_region = *new_region_or_error.value();
         auto& new_region = *new_region_or_error.value();

+ 1 - 0
Kernel/VM/Region.h

@@ -68,6 +68,7 @@ public:
 
 
     bool is_cacheable() const { return m_cacheable; }
     bool is_cacheable() const { return m_cacheable; }
     StringView name() const { return m_name ? m_name->view() : StringView {}; }
     StringView name() const { return m_name ? m_name->view() : StringView {}; }
+    OwnPtr<KString> take_name() { return move(m_name); }
     Region::Access access() const { return static_cast<Region::Access>(m_access); }
     Region::Access access() const { return static_cast<Region::Access>(m_access); }
 
 
     void set_name(OwnPtr<KString> name) { m_name = move(name); }
     void set_name(OwnPtr<KString> name) { m_name = move(name); }