Mmap fails for empty files on Windows

[NightRa]

As per the MSDN documentation for CreateFileMapping: An attempt to map a file with a length of 0 (zero) fails with an error code of ERROR_FILE_INVALID. Applications should test for files with a length of 0 (zero) and reject those files. I believe this should not be the expected behavior for the memmap-rs crate, and we should check if the size is 0 (we already get the size before mapping the file anyway), and return an empty-slice backed Mmap struct in that case.

[danburkert]

I believe this should not be the expected behavior for the memmap-rs crate, and we should check if the size is 0 (we already get the size before mapping the file anyway), and return an empty-slice backed Mmap struct in that case.

Can you expand on why you would prefer that? The behavior of returning an error is consistent on Windows and Linux, and is even checked in the test suite.

[oconnor663]

While the current behavior is consistent, I think almost all callers are going to need special handling for the zero-length case to avoid this error, and as a result there are probably a lot of latent bugs out there waiting for someone to trigger them with an empty file. I just realized I have one in my own code, where I check for a number of conditions but forgot to check for this one.

Maybe relevant, there are cases today where Mmap can accept a zero length and successfully deref to an empty slice. It can happen if you have an offset that isn't a multiple of the page size, so that the real mapping under the covers isn't empty. So in that particular case, the caller might end up with an even trickier bug, where their empty maps fail only for certain offsets. (This is especially tough because an "is the file empty" check isn't good enough here. You have to know how your request offset compares to the file length.)

[danburkert]

@oconnor663

Maybe relevant, there are cases today where Mmap can accept a zero length and successfully deref to an empty slice...

That's a very convincing argument, based on that line of reasoning I'd accept a PR to make the behavior consistent with explicit checks. Should this be considered a backwards incompatible change (and require a 0.8 version bump?)

[BurntSushi]

@danburkert I've generally considered the transformation of an error into a non-error to not be a breaking change---and have even designed APIs around that assumption to allow backwards compatible expansion in the future. I'm trying to think of any special considerations that might be in play here. The only thing I can think of is virtual files on Linux. My goto example is /proc/cpuinfo. I don't think you can memory map such files, so I rely on an error being returned when you try to memory map them, which then prompts a fallback strategy for using normal read calls. Looking at the behavior today, it looks like the error returned is the non-zero length error:

/proc/cpuinfo: failed to open memory map: memory map must have a non-zero length

So in that sense, if memory mapping virtual files now "works" by claiming it is an empty file, then you might consider that a breaking change. Tangentially, it does also seem less than ideal for opening such files to appear as if they succeed. If they do, then callers can't actually remove the zero length check anyway.

@oconnor663 I've generally found it more robust to just call mmap instead of trying to predict when it will fail. If it fails, then try a normal File::open call. If that succeeds, then return None, otherwise return an error.

[oconnor663]

Wow this is dirtier than I imagined :| And trying to distinguish virtual files based on the device type sounds like it would invite a lot of portability issues and platform-specific code that memmap probably doesn't want.

Would anyone consider this an acceptable strategy (very low confidence suggestion here): "If File::metadata reports a size of zero, attempt to read a single byte from the file. If the read succeeds, assume that the metadata is lying, and return an error indicating that the mapping failed. If the read returns EOF, trust the metadata, and return a Mmap/MmapMut object that derefs to the empty slice."

I can already think of one case that would break this logic, which would be to open a File and then read it to the end before passing it to memmap. Normally such a file handle should still result in a complete mapping of the file, ignoring the cursor position, but in this case it could trick the logic above into believing that /proc/cpuinfo was empty. A workaround for that case could be to seek(Start(0)) before reading, though I'd worry about 1) the performance cost of all these extra syscalls and 2) whether every mappable file descriptor is actually going to support seek. Maybe performance isn't a worry if we only do these steps for empty files? But there are so many caveats and assumptions here that I'm not feeling good about it.

[BurntSushi]

@oconnor663 Yeah, that does not sound pleasant. The extra syscall there does not seem good. AIUI, zero length memory maps are an error reported by the syscall itself, right? If so, then I kind of feel like we should just surface that as is done today I think.

[faulesocke]

Why so complicated? Just handle the case where the len parameter to mmap is 0, i.e. just change these lines: https://github.com/danburkert/memmap-rs/blob/3b047cc2b04558d8a1de3933be5f573c74bc8e0f/src/unix.rs#L40-L46

In this case you could safely do something like:

return Ok(MmapInner {
                    ptr: std::ptr::null_mut(),
                    len: 0,
});

Handling of special virtual files that are of length zero but still give some bytes during read()ing them should be up to the user. In no case I would expect from the memmap crate to do reads or seeks on the given File.

[BurntSushi]

Why so complicated?

The implementation is not what's complicated. Deciding the behavior in the first place is what's complicated.

Handling of special virtual files that are of length zero but still give some bytes during read()ing them should be up to the user.

This is kind of just dismissing the point. The idea, as far as I can tell, was to reduce the case analysis the caller is required to perform in order to use mmap correctly. But this just winds up shifting the case analysis rather than removing it; and moreover, is arguably a breaking change.

[faulesocke]

I see the difficulty here but I think that proper detection and handling of unmappable files should be left to the user.

[BurntSushi]

I see the difficulty here but I think that proper detection and handling of unmappable files should be left to the user.

That's the case today. From man mmap on my Linux system:

If len is zero, mmap() shall fail and no mapping shall be established.

[faulesocke]

According to the man pages the standard way to check for filesystems without mmap support seems to be to call mmap on these and see if it fails with ENODEV. However, this would collide with the autosize-code from MmapOptions. I think there are multiple options:

1. Require the user to explicitly set the mapping length if the file is 0-sized
2. Always require the user to set the mapping length and offer an explizit "use file size as mapping size" option, i.e. make the size autodetection explicit
3. Just ignore this entirely and simply rely on the user that he does not try to mmap special files from filesystems which do not support mmap. This might work good enough since programs operating on filesystems not supporting mmap are usually fundamentally different from programs that operate on "regular" filesystems.

I would prefer option 2 even though it is a breaking change.

References: From the Linux Programmer's Manual:

ENODEV The underlying filesystem of the specified file does not support memory mapping.

From the POSIX Programmer's Manual:

ENODEV The fildes argument refers to a file whose type is not supported by mmap()

[oconnor663]

Just ignore this entirely and simply rely on the user that he does not try to mmap special files from filesystems which do not support mmap.

This doesn't work well for programs that traverse the entire file hierarchy, like rg or tar. It implies that the user may not invoke the program from /, but that's something plenty of users want to do. I'm sure there are wacky examples of virtual filesystems in places other than /, too, but that's bad enough.

It seems like the underlying APIs are pretty tightly coupled to the notion that memmapping empty files is an error, and that callers will fall back to normal reading in the face of errors. (Or perhaps that callers won't try to mmap below a certain size.) If we want to make empty files not an error, it's suddenly our responsibility to detect when the filesystem is lying to use about a file's size. And as the old saying goes: If the file system wants to lie to us, then let the file system lie to us.

[faulesocke]

It seems like the underlying APIs are pretty tightly coupled to the notion that memmapping empty files is an error

Mapping empty files isn't an error, you can safely map an empty file as long as the mapping is larger than the file. You can not write behind the files actual end, but mapping is still possible and once you increase the file size writing becomes safe.

If the file system wants to lie to us

And the file system is also not really lying to us (this may look like a detail but is important): For example the procfs reports size 0 for /proc/cpuinfo. This is because the "file" does not actually take up any size anywhere - it is dynamically generated while read() ing it. Therefore mmap is not possible on this file. There are other files in /proc, for example you might find config.gz. The content of this file is compiled into the procfs source code and therefore takes up memory while the system is running and is therefore not 0-sized, even though it lies on a "virtual" file system.

A file in Linux is simply just a handle to a certain resource, in the case of /proc/cpuinfo a handle towards cpu information. The actual resource defines how you may access it - in this case only via the read() syscall.

So: mapping a 0-sized file is fine as long as the mapping is larger than 0. Mapping unmappable files (/proc/cpuinfo is likely such a file, but I haven't tried yet) will yield an error. A file is unmappable if the filesystem decides not to implement mmap at all or if the filesystem decides that this specific file is not mappable.

[faulesocke]

Well, while thinking about it, this offers another possible solution: Always internally map at least one page of memory, even when the file is actually smaller. If the mmap returns an error here, we know that a file is not mappable, if it succeeds, we are fine and can still give an empty slice to the user.

[oconnor663]

Always internally map at least one page of memory, even when the file is actually smaller.

Oh I like that idea. Are there any cases where stat reports 0 size, mmap succeeds, but nonetheless read would have returned some bytes? Other than race conditions.

[faulesocke]

Yes, for example many of the device files in /dev. You can mmap() them, you can read() them, you can even read from the map but stat shows size 0.

[oconnor663]

What's the Right Way to mmap a device like that? How do you know how many bytes to map?

[faulesocke]

AFAIK you get the size of the underlying storage via ioctl calls. But to make those calls you need the apropriate access rights. I recommend that you don't think of device files like regular files. Only because some of them behave a lot like regular files, most of them won't and will still be mappable. For example imagine a CD changer. It may have multiple CDs inside, and you can set the active one through ioctl calls. The size of the device is unclear or can change at any time. So the device file really does not have a size but instead the underlying hardware may have one or more sizes. The file is just an abstraction over it.

[SimonSapin]

Mapping empty files isn't an error, you can safely map an empty file as long as the mapping is larger than the file.

I’ve implemented this approach in https://github.com/RazrFalcon/memmap2-rs/pull/25. Unfortunately the same idea does not seem to work on Windows: https://github.com/RazrFalcon/memmap2-rs/pull/25#issuecomment-920779236

Looking at the behavior today, it looks like the error returned is the non-zero length error:

/proc/cpuinfo: failed to open memory map: memory map must have a non-zero length

So in that sense, if memory mapping virtual files now "works" by claiming it is an empty file, then you might consider that a breaking change.

With my PR on my Linux machine, trying to map /proc/cpuinfo still returns an error but it’s an IoError::Os { code: 13, kind: PermissionDenied, message: "Permission denied" }.

[SimonSapin]

Mapping empty files isn't an error, you can safely map an empty file as long as the mapping is larger than the file.

@faulesocke This happens to work with https://github.com/RazrFalcon/memmap2-rs/pull/25 on my machine with these particular kernel and glibc versions, but do you know of documentation that supports this pattern as a correct use of mmap? I don’t find it in https://www.man7.org/linux/man-pages/man2/mmap.2.html

(All that I find is discussion of the bytes between the end of the file and the next page boundary, but zero is already page-aligned so not relevant here.)

Python’s mmap.mmap raises ValueError: mmap length is greater than file size (presumably doing its own checks before calling libc’s mmap)

[SimonSapin]

Ah! I think I found it in the POSIX definition:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/mmap.html

The mmap() function can be used to map a region of memory that is larger than the current size of the object.

It goes on:

Memory access within the mapping but beyond the current end of the underlying objects may result in SIGBUS signals being sent to the process. The reason for this is that the size of the object can be manipulated by other processes and can change at any moment. The implementation should tell the application that a memory reference is outside the object where this can be detected; otherwise, written data may be lost and read data may not reflect actual data in the object.

So reading from or writing to the area mapped past the end of the file can cause SIGBUS but the existence of that area is fine.