Kernel: Implement proper memory region splitting

Memory regions are now splitted when they get munmapped, mprotected, or
mmapped with MAP_FIXED. This is used by couple of ports, and without
this we were just leaking up memory or straight up crashing programs.
This commit is contained in:
Bananymous 2025-11-10 19:57:26 +02:00
parent a39aa73e21
commit 9537922acc
10 changed files with 145 additions and 46 deletions

View File

@ -33,6 +33,7 @@ namespace Kernel
BAN::ErrorOr<void> msync(vaddr_t, size_t, int) override; BAN::ErrorOr<void> msync(vaddr_t, size_t, int) override;
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> clone(PageTable& new_page_table) override; BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> clone(PageTable& new_page_table) override;
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> split(size_t offset) override;
protected: protected:
BAN::ErrorOr<bool> allocate_page_containing_impl(vaddr_t vaddr, bool wants_write) override; BAN::ErrorOr<bool> allocate_page_containing_impl(vaddr_t vaddr, bool wants_write) override;

View File

@ -15,6 +15,7 @@ namespace Kernel
~MemoryBackedRegion(); ~MemoryBackedRegion();
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> clone(PageTable& new_page_table) override; BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> clone(PageTable& new_page_table) override;
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> split(size_t offset) override;
BAN::ErrorOr<void> msync(vaddr_t, size_t, int) override { return {}; } BAN::ErrorOr<void> msync(vaddr_t, size_t, int) override { return {}; }

View File

@ -60,6 +60,7 @@ namespace Kernel
BAN::ErrorOr<bool> allocate_page_containing(vaddr_t address, bool wants_write); BAN::ErrorOr<bool> allocate_page_containing(vaddr_t address, bool wants_write);
virtual BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> clone(PageTable& new_page_table) = 0; virtual BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> clone(PageTable& new_page_table) = 0;
virtual BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> split(size_t offset) = 0;
protected: protected:
MemoryRegion(PageTable&, size_t size, Type type, PageTable::flags_t flags, int status_flags); MemoryRegion(PageTable&, size_t size, Type type, PageTable::flags_t flags, int status_flags);
@ -69,7 +70,7 @@ namespace Kernel
protected: protected:
PageTable& m_page_table; PageTable& m_page_table;
const size_t m_size; size_t m_size { 0 };
const Type m_type; const Type m_type;
PageTable::flags_t m_flags; PageTable::flags_t m_flags;
const int m_status_flags; const int m_status_flags;

View File

@ -58,6 +58,7 @@ namespace Kernel
static BAN::ErrorOr<BAN::UniqPtr<SharedMemoryObject>> create(BAN::RefPtr<SharedMemoryObjectManager::Object>, PageTable&, AddressRange); static BAN::ErrorOr<BAN::UniqPtr<SharedMemoryObject>> create(BAN::RefPtr<SharedMemoryObjectManager::Object>, PageTable&, AddressRange);
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> clone(PageTable& new_page_table) override; BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> clone(PageTable& new_page_table) override;
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> split(size_t offset) override;
BAN::ErrorOr<void> msync(vaddr_t, size_t, int) override { return {}; } BAN::ErrorOr<void> msync(vaddr_t, size_t, int) override { return {}; }

View File

@ -172,6 +172,7 @@ namespace Kernel
BAN::ErrorOr<long> sys_readdir(int fd, struct dirent* list, size_t list_len); BAN::ErrorOr<long> sys_readdir(int fd, struct dirent* list, size_t list_len);
BAN::ErrorOr<BAN::Vector<BAN::UniqPtr<MemoryRegion>>> split_memory_region(BAN::UniqPtr<MemoryRegion>&& region, vaddr_t base, size_t length);
BAN::ErrorOr<long> sys_mmap(const sys_mmap_t*); BAN::ErrorOr<long> sys_mmap(const sys_mmap_t*);
BAN::ErrorOr<long> sys_munmap(void* addr, size_t len); BAN::ErrorOr<long> sys_munmap(void* addr, size_t len);
BAN::ErrorOr<long> sys_mprotect(void* addr, size_t len, int prot); BAN::ErrorOr<long> sys_mprotect(void* addr, size_t len, int prot);

View File

@ -301,6 +301,13 @@ namespace Kernel
return BAN::UniqPtr<MemoryRegion>(BAN::move(region)); return BAN::UniqPtr<MemoryRegion>(BAN::move(region));
} }
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> split(size_t offset) override
{
(void)offset;
dwarnln("TODO: FramebufferMemoryRegion::split");
return BAN::Error::from_errno(ENOTSUP);
}
protected: protected:
// Returns error if no memory was available // Returns error if no memory was available
// Returns true if page was succesfully allocated // Returns true if page was succesfully allocated

View File

@ -232,4 +232,33 @@ namespace Kernel
return BAN::UniqPtr<MemoryRegion>(BAN::move(result)); return BAN::UniqPtr<MemoryRegion>(BAN::move(result));
} }
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> FileBackedRegion::split(size_t offset)
{
ASSERT(offset && offset < m_size);
ASSERT(offset % PAGE_SIZE == 0);
const bool has_dirty_pages = (m_type == Type::PRIVATE);
BAN::Vector<paddr_t> dirty_pages;
if (has_dirty_pages)
{
TRY(dirty_pages.resize(BAN::Math::div_round_up<size_t>(m_size - offset, PAGE_SIZE)));
for (size_t i = 0; i < dirty_pages.size(); i++)
dirty_pages[i] = m_dirty_pages[i + offset / PAGE_SIZE];
}
auto* new_region = new FileBackedRegion(m_inode, m_page_table, m_offset + offset, m_size - offset, m_type, m_flags, m_status_flags);
if (new_region == nullptr)
return BAN::Error::from_errno(ENOTSUP);
new_region->m_vaddr = m_vaddr + offset;
new_region->m_shared_data = m_shared_data;
new_region->m_dirty_pages = BAN::move(dirty_pages);
m_size = offset;
if (has_dirty_pages)
MUST(m_dirty_pages.resize(offset / PAGE_SIZE));
return BAN::UniqPtr<MemoryRegion>::adopt(new_region);
}
} }

View File

@ -82,6 +82,21 @@ namespace Kernel
return BAN::UniqPtr<MemoryRegion>(BAN::move(result)); return BAN::UniqPtr<MemoryRegion>(BAN::move(result));
} }
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> MemoryBackedRegion::split(size_t offset)
{
ASSERT(offset && offset < m_size);
ASSERT(offset % PAGE_SIZE == 0);
auto* new_region = new MemoryBackedRegion(m_page_table, m_size - offset, m_type, m_flags, m_status_flags);
if (new_region == nullptr)
return BAN::Error::from_errno(ENOMEM);
new_region->m_vaddr = m_vaddr + offset;
m_size = offset;
return BAN::UniqPtr<MemoryRegion>::adopt(new_region);
}
BAN::ErrorOr<void> MemoryBackedRegion::copy_data_to_region(size_t offset_into_region, const uint8_t* buffer, size_t buffer_size) BAN::ErrorOr<void> MemoryBackedRegion::copy_data_to_region(size_t offset_into_region, const uint8_t* buffer, size_t buffer_size)
{ {
ASSERT(offset_into_region + buffer_size <= m_size); ASSERT(offset_into_region + buffer_size <= m_size);

View File

@ -87,6 +87,13 @@ namespace Kernel
return BAN::UniqPtr<MemoryRegion>(BAN::move(region)); return BAN::UniqPtr<MemoryRegion>(BAN::move(region));
} }
BAN::ErrorOr<BAN::UniqPtr<MemoryRegion>> SharedMemoryObject::split(size_t offset)
{
(void)offset;
dwarnln("TODO: SharedMemoryObject::split");
return BAN::Error::from_errno(ENOTSUP);
}
BAN::ErrorOr<bool> SharedMemoryObject::allocate_page_containing_impl(vaddr_t address, bool wants_write) BAN::ErrorOr<bool> SharedMemoryObject::allocate_page_containing_impl(vaddr_t address, bool wants_write)
{ {
ASSERT(contains(address)); ASSERT(contains(address));

View File

@ -2189,6 +2189,38 @@ namespace Kernel
return 0; return 0;
} }
BAN::ErrorOr<BAN::Vector<BAN::UniqPtr<MemoryRegion>>> Process::split_memory_region(BAN::UniqPtr<MemoryRegion>&& region, vaddr_t base, size_t length)
{
ASSERT(base % PAGE_SIZE == 0);
ASSERT(base < region->vaddr() + region->size());
if (auto rem = length % PAGE_SIZE)
length += PAGE_SIZE - rem;
if (base + length > region->vaddr() + region->size())
length = region->vaddr() + region->size() - base;
BAN::Vector<BAN::UniqPtr<MemoryRegion>> result;
TRY(result.reserve(3));
if (region->vaddr() < base)
{
auto temp = TRY(region->split(base - region->vaddr()));
MUST(result.push_back(BAN::move(region)));
region = BAN::move(temp);
}
if (base + length < region->vaddr() + region->size())
{
auto temp = TRY(region->split(base + length - region->vaddr()));
MUST(result.push_back(BAN::move(region)));
region = BAN::move(temp);
}
MUST(result.push_back(BAN::move(region)));
return result;
}
BAN::ErrorOr<long> Process::sys_mmap(const sys_mmap_t* user_args) BAN::ErrorOr<long> Process::sys_mmap(const sys_mmap_t* user_args)
{ {
sys_mmap_t args; sys_mmap_t args;
@ -2201,6 +2233,9 @@ namespace Kernel
if (args.prot != PROT_NONE && (args.prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC))) if (args.prot != PROT_NONE && (args.prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC)))
return BAN::Error::from_errno(EINVAL); return BAN::Error::from_errno(EINVAL);
if (!(args.flags & MAP_ANONYMOUS) && (args.off % PAGE_SIZE))
return BAN::Error::from_errno(EINVAL);
if (!(args.flags & MAP_PRIVATE) == !(args.flags & MAP_SHARED)) if (!(args.flags & MAP_PRIVATE) == !(args.flags & MAP_SHARED))
return BAN::Error::from_errno(EINVAL); return BAN::Error::from_errno(EINVAL);
auto region_type = (args.flags & MAP_PRIVATE) ? MemoryRegion::Type::PRIVATE : MemoryRegion::Type::SHARED; auto region_type = (args.flags & MAP_PRIVATE) ? MemoryRegion::Type::PRIVATE : MemoryRegion::Type::SHARED;
@ -2241,10 +2276,18 @@ namespace Kernel
{ {
if (!m_mapped_regions[i]->overlaps(vaddr, args.len)) if (!m_mapped_regions[i]->overlaps(vaddr, args.len))
continue; continue;
if (!m_mapped_regions[i]->is_contained_by(vaddr, args.len))
derrorln("VERY BROKEN MAP_FIXED UNMAP");
m_mapped_regions[i]->wait_not_pinned(); m_mapped_regions[i]->wait_not_pinned();
auto temp = BAN::move(m_mapped_regions[i]);
m_mapped_regions.remove(i--); m_mapped_regions.remove(i--);
if (!temp->is_contained_by(vaddr, args.len))
{
auto new_regions = TRY(split_memory_region(BAN::move(temp), vaddr, args.len));
for (auto& new_region : new_regions)
if (!new_region->overlaps(vaddr, args.len))
TRY(m_mapped_regions.push_back(BAN::move(new_region)));
}
} }
} }
else if (const vaddr_t vaddr = reinterpret_cast<vaddr_t>(args.addr); vaddr == 0) else if (const vaddr_t vaddr = reinterpret_cast<vaddr_t>(args.addr); vaddr == 0)
@ -2267,9 +2310,6 @@ namespace Kernel
if (args.flags & MAP_ANONYMOUS) if (args.flags & MAP_ANONYMOUS)
{ {
if (args.off != 0)
return BAN::Error::from_errno(EINVAL);
auto region = TRY(MemoryBackedRegion::create( auto region = TRY(MemoryBackedRegion::create(
page_table(), page_table(),
args.len, args.len,
@ -2330,7 +2370,7 @@ namespace Kernel
if (auto rem = vaddr % PAGE_SIZE) if (auto rem = vaddr % PAGE_SIZE)
{ {
vaddr -= rem; vaddr -= rem;
len += PAGE_SIZE - rem; len += rem;
} }
if (auto rem = len % PAGE_SIZE) if (auto rem = len % PAGE_SIZE)
@ -2338,22 +2378,22 @@ namespace Kernel
LockGuard _(m_process_lock); LockGuard _(m_process_lock);
// FIXME: We should unmap partial regions.
// This is a hack to only unmap if the whole mmap region
// is contained within [addr, addr + len]
for (size_t i = 0; i < m_mapped_regions.size(); i++) for (size_t i = 0; i < m_mapped_regions.size(); i++)
{ {
auto& region = m_mapped_regions[i]; if (!m_mapped_regions[i]->overlaps(vaddr, len))
continue;
const vaddr_t region_s = region->vaddr(); m_mapped_regions[i]->wait_not_pinned();
const vaddr_t region_e = region->vaddr() + region->size(); auto temp = BAN::move(m_mapped_regions[i]);
if (vaddr <= region_s && region_e <= vaddr + len)
{
region->wait_not_pinned();
m_mapped_regions.remove(i--); m_mapped_regions.remove(i--);
if (!temp->is_contained_by(vaddr, len))
{
auto new_regions = TRY(split_memory_region(BAN::move(temp), vaddr, len));
for (auto& new_region : new_regions)
if (!new_region->overlaps(vaddr, len))
TRY(m_mapped_regions.push_back(BAN::move(new_region)));
} }
else if (region->overlaps(vaddr, len))
dwarnln("TODO: partial region munmap");
} }
return 0; return 0;
@ -2386,39 +2426,35 @@ namespace Kernel
LockGuard _(m_process_lock); LockGuard _(m_process_lock);
// FIXME: We should protect partial regions.
// This is a hack to only protect if the whole mmap region
// is contained within [addr, addr + len]
for (size_t i = 0; i < m_mapped_regions.size(); i++) for (size_t i = 0; i < m_mapped_regions.size(); i++)
{ {
if (!m_mapped_regions[i]->overlaps(vaddr, len))
continue;
if (!m_mapped_regions[i]->is_contained_by(vaddr, len))
{
m_mapped_regions[i]->wait_not_pinned();
auto temp = BAN::move(m_mapped_regions[i]);
m_mapped_regions.remove(i--);
auto new_regions = TRY(split_memory_region(BAN::move(temp), vaddr, len));
for (auto& new_region : new_regions)
TRY(m_mapped_regions.push_back(BAN::move(new_region)));
continue;
}
auto& region = m_mapped_regions[i]; auto& region = m_mapped_regions[i];
const vaddr_t region_s = region->vaddr();
const vaddr_t region_e = region->vaddr() + region->size();
if (vaddr <= region_s && region_e <= vaddr + len)
{
const bool is_shared = (region->type() == MemoryRegion::Type::SHARED); const bool is_shared = (region->type() == MemoryRegion::Type::SHARED);
const bool is_writable = (region->status_flags() & O_WRONLY); const bool is_writable = (region->status_flags() & O_WRONLY);
const bool want_write = (prot & PROT_WRITE); const bool want_write = (prot & PROT_WRITE);
if (is_shared && want_write && !is_writable) if (is_shared && want_write && !is_writable)
return BAN::Error::from_errno(EACCES); return BAN::Error::from_errno(EACCES);
// FIXME: if the region is pinned writable, this may // NOTE: don't change protection of regions in use
// cause some problems :D region->wait_not_pinned();
TRY(region->mprotect(flags)); TRY(region->mprotect(flags));
} }
else if (region->overlaps(vaddr, len))
{
const bool is_shared = (region->type() == MemoryRegion::Type::SHARED);
const bool is_writable = (region->status_flags() & O_WRONLY);
const bool want_write = (prot & PROT_WRITE);
if (is_shared && want_write && !is_writable)
return BAN::Error::from_errno(EACCES);
dwarnln("TODO: partial region mprotect");
TRY(region->mprotect(flags | region->flags()));
}
}
return 0; return 0;
} }