#18733 closed bug (fixed)

mmap fails with out of memory if map big read-only file with MAP_PRIVATE flag

Reported by: X512 Owned by: nobody
Priority: normal Milestone: R1/beta5
Component: System/Kernel Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description

This is hrev57493.

map_file syscall used in mmap implementation fails to map read-only file on disk if mapped size is larger than total amount of RAM. It cause Git to fail to fetch big repositories:

> git fetch origin-new
remote: Enumerating objects: 433106, done.
remote: Counting objects: 100% (175641/175641), done.
remote: Total 433106 (delta 175641), reused 175641 (delta 175641), pack-reused 257465
Receiving objects: 100% (433106/433106), 992.65 MiB | 3.78 MiB/s, done.
fatal: packfile .git/objects/pack/pack-ab36c9a68f870060114341fac8c873d549467667.pack cannot be mapped: Out of memory
fatal: fetch-pack: invalid index-pack output

Changing MAP_PRIVATE flag to MAP_SHARED fixes problem and Git fetch succeeds.

As I understand there are no difference between MAP_PRIVATE and MAP_SHARED if file is mapped without write access.

Quick patch that fixes issue:

  • src/system/libroot/posix/sys/mman.cpp

    diff --git a/src/system/libroot/posix/sys/mman.cpp b/src/system/libroot/posix/sys/mman.cpp
    index f91771a7ef..2d7ad640d5 100644
    a b mmap(void* address, size_t length, int protection, int flags, int fd,  
    125125       }
    126126
    127127       // translate mapping, address specification, and protection
    128        int mapping = (flags & MAP_SHARED) != 0
     128       int mapping = (flags & MAP_SHARED) != 0 || (protection & PROT_WRITE) == 0
    129129               ? REGION_NO_PRIVATE_MAP : REGION_PRIVATE_MAP;
    130130
    131131       uint32 addressSpec;

Attachments (1)

mmap_private_mprotect.cpp (1.4 KB ) - added by waddlesplash 10 months ago.

Download all attachments as: .zip

Change History (6)

comment:1 by waddlesplash, 11 months ago

I think this optimization needs to be done in the kernel, because I think it's checked whether the FD was opened read-write or read-only? If the FD was opened read-only, then yes, we can use MAP_SHARED, I think. But I'd have to check.

comment:2 by waddlesplash, 10 months ago

As I understand there are no difference between MAP_PRIVATE and MAP_SHARED if file is mapped without write access.

This is true, however, files mapped without MAP_SHARED (i.e. with REGION_PRIVATE_MAP) can have PROT_WRITE added later on (via mprotect), after mmap was called, and all will work as expected. If we were to optimize this inside mmap or in the kernel, then that would prevent applications from doing that.

So, I don't think we can make this change either in libroot or in the kernel directly.

Instead, what we could do is to avoid committing memory in this case, and then only commit it when mprotect() was called (and thus fail in mprotect if memory can't be reserved.)

It looks like set_area_protection already alters commitment on changing from non-writable -> writable, but mprotect doesn't.

comment:3 by X512, 10 months ago

files mapped without MAP_SHARED (i.e. with REGION_PRIVATE_MAP) can have PROT_WRITE added later on (via mprotect)

I suspect mprotect should fail with out of memory in this case.

Version 0, edited 10 months ago by X512 (next)

by waddlesplash, 10 months ago

Attachment: mmap_private_mprotect.cpp added

comment:4 by waddlesplash, 10 months ago

Test case attached that reproduces this problem (requires creating a file larger than system RAM for testing.)

Proposed fix: https://review.haiku-os.org/c/haiku/+/7339

comment:5 by waddlesplash, 10 months ago

Milestone: UnscheduledR1/beta5
Resolution: fixed
Status: newclosed

Fixed in hrev57517.

Note: See TracTickets for help on using tickets.