Kernel: Implement basic support for sys$mmap() with MAP_PRIVATE

You can now mmap a file as private and writable, and the changes you make will only be visible to you. This works because internally a MAP_PRIVATE region is backed by a unique PrivateInodeVMObject instead of using the globally shared SharedInodeVMObject like we always did before. :^) Fixes SerenityOS#1045.
cubiclove · Feb 28, 2020 · 8fbdda5 · 8fbdda5
1 parent aa1e209
commit 8fbdda5
Show file tree

Hide file tree

Showing 11 changed files with 44 additions and 30 deletions.
diff --git a/Kernel/Devices/BXVGADevice.cpp b/Kernel/Devices/BXVGADevice.cpp
@@ -166,9 +166,11 @@ u32 BXVGADevice::find_framebuffer_address()
  return framebuffer_address;
 }
 
-KResultOr<Region*> BXVGADevice::mmap(Process& process, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot)
+KResultOr<Region*> BXVGADevice::mmap(Process& process, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared)
 {
  REQUIRE_PROMISE(video);
+ if (!shared)
+ return KResult(-ENODEV);
  ASSERT(offset == 0);
  ASSERT(size == framebuffer_size_in_bytes());
  auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes());

diff --git a/Kernel/Devices/BXVGADevice.h b/Kernel/Devices/BXVGADevice.h
@@ -41,7 +41,7 @@ class BXVGADevice final : public BlockDevice {
  BXVGADevice();
 
  virtual int ioctl(FileDescription&, unsigned request, unsigned arg) override;
- virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t, int prot) override;
+ virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t, int prot, bool shared) override;
 
 private:
  virtual const char* class_name() const override { return "BXVGA"; }

diff --git a/Kernel/Devices/MBVGADevice.cpp b/Kernel/Devices/MBVGADevice.cpp
@@ -51,9 +51,11 @@ MBVGADevice::MBVGADevice(PhysicalAddress addr, int pitch, int width, int height)
  s_the = this;
 }
 
-KResultOr<Region*> MBVGADevice::mmap(Process& process, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot)
+KResultOr<Region*> MBVGADevice::mmap(Process& process, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared)
 {
  REQUIRE_PROMISE(video);
+ if (!shared)
+ return KResult(-ENODEV);
  ASSERT(offset == 0);
  ASSERT(size == framebuffer_size_in_bytes());
  auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes());

diff --git a/Kernel/Devices/MBVGADevice.h b/Kernel/Devices/MBVGADevice.h
@@ -41,7 +41,7 @@ class MBVGADevice final : public BlockDevice {
  MBVGADevice(PhysicalAddress addr, int pitch, int width, int height);
 
  virtual int ioctl(FileDescription&, unsigned request, unsigned arg) override;
- virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t, int prot) override;
+ virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t, int prot, bool shared) override;
 
 private:
  virtual const char* class_name() const override { return "MBVGA"; }

diff --git a/Kernel/FileSystem/File.cpp b/Kernel/FileSystem/File.cpp
@@ -54,7 +54,7 @@ int File::ioctl(FileDescription&, unsigned, unsigned)
  return -ENOTTY;
 }
 
-KResultOr<Region*> File::mmap(Process&, FileDescription&, VirtualAddress, size_t, size_t, int)
+KResultOr<Region*> File::mmap(Process&, FileDescription&, VirtualAddress, size_t, size_t, int, bool)
 {
  return KResult(-ENODEV);
 }

diff --git a/Kernel/FileSystem/File.h b/Kernel/FileSystem/File.h
@@ -77,7 +77,7 @@ class File : public RefCounted<File> {
  virtual ssize_t read(FileDescription&, u8*, ssize_t) = 0;
  virtual ssize_t write(FileDescription&, const u8*, ssize_t) = 0;
  virtual int ioctl(FileDescription&, unsigned request, unsigned arg);
- virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot);
+ virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared);
 
  virtual String absolute_path(const FileDescription&) const = 0;
 

diff --git a/Kernel/FileSystem/FileDescription.cpp b/Kernel/FileSystem/FileDescription.cpp
@@ -281,10 +281,10 @@ InodeMetadata FileDescription::metadata() const
  return {};
 }
 
-KResultOr<Region*> FileDescription::mmap(Process& process, VirtualAddress vaddr, size_t offset, size_t size, int prot)
+KResultOr<Region*> FileDescription::mmap(Process& process, VirtualAddress vaddr, size_t offset, size_t size, int prot, bool shared)
 {
  LOCKER(m_lock);
- return m_file->mmap(process, *this, vaddr, offset, size, prot);
+ return m_file->mmap(process, *this, vaddr, offset, size, prot, shared);
 }
 
 KResult FileDescription::truncate(u64 length)

diff --git a/Kernel/FileSystem/FileDescription.h b/Kernel/FileSystem/FileDescription.h
@@ -109,7 +109,7 @@ class FileDescription : public RefCounted<FileDescription> {
  Custody* custody() { return m_custody.ptr(); }
  const Custody* custody() const { return m_custody.ptr(); }
 
- KResultOr<Region*> mmap(Process&, VirtualAddress, size_t offset, size_t, int prot);
+ KResultOr<Region*> mmap(Process&, VirtualAddress, size_t offset, size_t, int prot, bool shared);
 
  bool is_blocking() const { return m_is_blocking; }
  void set_blocking(bool b) { m_is_blocking = b; }

diff --git a/Kernel/FileSystem/InodeFile.cpp b/Kernel/FileSystem/InodeFile.cpp
@@ -29,6 +29,7 @@
 #include <Kernel/FileSystem/InodeFile.h>
 #include <Kernel/FileSystem/VirtualFileSystem.h>
 #include <Kernel/Process.h>
+#include <Kernel/VM/PrivateInodeVMObject.h>
 #include <Kernel/VM/SharedInodeVMObject.h>
 
 namespace Kernel {
@@ -60,11 +61,18 @@ ssize_t InodeFile::write(FileDescription& description, const u8* data, ssize_t c
  return nwritten;
 }
 
-KResultOr<Region*> InodeFile::mmap(Process& process, FileDescription& description, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot)
+KResultOr<Region*> InodeFile::mmap(Process& process, FileDescription& description, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared)
 {
  ASSERT(offset == 0);
  // FIXME: If PROT_EXEC, check that the underlying file system isn't mounted noexec.
- auto* region = process.allocate_region_with_vmobject(preferred_vaddr, size, SharedInodeVMObject::create_with_inode(inode()), offset, description.absolute_path(), prot);
+ RefPtr<InodeVMObject> vmobject;
+ if (shared)
+ vmobject = SharedInodeVMObject::create_with_inode(inode());
+ else
+ vmobject = PrivateInodeVMObject::create_with_inode(inode());
+ if (!vmobject)
+ return KResult(-ENOMEM);
+ auto* region = process.allocate_region_with_vmobject(preferred_vaddr, size, *vmobject, offset, description.absolute_path(), prot);
  if (!region)
  return KResult(-ENOMEM);
  return region;

diff --git a/Kernel/FileSystem/InodeFile.h b/Kernel/FileSystem/InodeFile.h
@@ -49,7 +49,7 @@ class InodeFile final : public File {
 
  virtual ssize_t read(FileDescription&, u8*, ssize_t) override;
  virtual ssize_t write(FileDescription&, const u8*, ssize_t) override;
- virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot) override;
+ virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared) override;
 
  virtual String absolute_path(const FileDescription&) const override;
 

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
@@ -321,19 +321,22 @@ static bool validate_mmap_prot(int prot, bool map_stack)
  return true;
 }
 
-static bool validate_inode_mmap_prot(const Process& process, int prot, const Inode& inode)
+static bool validate_inode_mmap_prot(const Process& process, int prot, const Inode& inode, bool map_shared)
 {
  auto metadata = inode.metadata();
- if ((prot & PROT_WRITE) && !metadata.may_write(process))
- return false;
  if ((prot & PROT_READ) && !metadata.may_read(process))
  return false;
- InterruptDisabler disabler;
- if (inode.shared_vmobject()) {
- if ((prot & PROT_EXEC) && inode.shared_vmobject()->writable_mappings())
- return false;
- if ((prot & PROT_WRITE) && inode.shared_vmobject()->executable_mappings())
+
+ if (map_shared) {
+ if ((prot & PROT_WRITE) && !metadata.may_write(process))
  return false;
+ InterruptDisabler disabler;
+ if (inode.shared_vmobject()) {
+ if ((prot & PROT_EXEC) && inode.shared_vmobject()->writable_mappings())
+ return false;
+ if ((prot & PROT_WRITE) && inode.shared_vmobject()->executable_mappings())
+ return false;
+ }
  }
  return true;
 }
@@ -433,28 +436,27 @@ void* Process::sys$mmap(const Syscall::SC_mmap_params* user_params)
  return (void*)-EINVAL;
  if (static_cast<size_t>(offset) & ~PAGE_MASK)
  return (void*)-EINVAL;
- // FIXME: Implement MAP_PRIVATE for FileDescription-backed mmap
- if (map_private)
- return (void*)-ENOTSUP;
  auto description = file_description(fd);
  if (!description)
  return (void*)-EBADF;
  if (description->is_directory())
  return (void*)-ENODEV;
  if ((prot & PROT_READ) && !description->is_readable())
  return (void*)-EACCES;
- if ((prot & PROT_WRITE) && !description->is_writable())
- return (void*)-EACCES;
+ if (map_shared) {
+ if ((prot & PROT_WRITE) && !description->is_writable())
+ return (void*)-EACCES;
+ }
  if (description->inode()) {
- if (!validate_inode_mmap_prot(*this, prot, *description->inode()))
+ if (!validate_inode_mmap_prot(*this, prot, *description->inode(), map_shared))
  return (void*)-EACCES;
  }
- auto region_or_error = description->mmap(*this, VirtualAddress(addr), static_cast<size_t>(offset), size, prot);
+ auto region_or_error = description->mmap(*this, VirtualAddress(addr), static_cast<size_t>(offset), size, prot, map_shared);
  if (region_or_error.is_error()) {
  // Fail if MAP_FIXED or address is 0, retry otherwise
  if (map_fixed || addr == 0)
  return (void*)(int)region_or_error.error();
- region_or_error = description->mmap(*this, {}, static_cast<size_t>(offset), size, prot);
+ region_or_error = description->mmap(*this, {}, static_cast<size_t>(offset), size, prot, map_shared);
  }
  if (region_or_error.is_error())
  return (void*)(int)region_or_error.error();
@@ -537,7 +539,7 @@ int Process::sys$mprotect(void* addr, size_t size, int prot)
  if (whole_region->access() == prot_to_region_access_flags(prot))
  return 0;
  if (whole_region->vmobject().is_inode()
- && !validate_inode_mmap_prot(*this, prot, static_cast<const SharedInodeVMObject&>(whole_region->vmobject()).inode())) {
+ && !validate_inode_mmap_prot(*this, prot, static_cast<const SharedInodeVMObject&>(whole_region->vmobject()).inode(), whole_region->is_shared())) {
  return -EACCES;
  }
  whole_region->set_readable(prot & PROT_READ);
@@ -556,7 +558,7 @@ int Process::sys$mprotect(void* addr, size_t size, int prot)
  if (old_region->access() == prot_to_region_access_flags(prot))
  return 0;
  if (old_region->vmobject().is_inode()
- && !validate_inode_mmap_prot(*this, prot, static_cast<const SharedInodeVMObject&>(old_region->vmobject()).inode())) {
+ && !validate_inode_mmap_prot(*this, prot, static_cast<const SharedInodeVMObject&>(old_region->vmobject()).inode(), old_region->is_shared())) {
  return -EACCES;
  }