How unsafe is mmap?

Yes, exactly. What I am trying to say is that questions about volatile or atomic access are not the core of the issue, it is whether mmap can actually be used to acquire a &[u8] to the underlying memory with the guarantees that implies for Rust. It seems that in general it currently cannot. The questions about atomic and volatile are equally valid if it would yield *const u8 and apply equally to C/C++, but I am interested in that particular extra bit of semantics that Rust adds to the memory reference.

I think that there is also one important difference to using unsafe code and writing to the slice: A completely safe function use_buf_as_str_and_write_junk_to_file(buf: &[u8], file: File) could show this behavior. And since it cannot know whether buf is memory-mapped (and it should not need to since all byte slices must have the same properties), getting a byte slice out of mmap in the first place seems to be the problem.

Also since the manual page of mmap actually says

It is unspecified whether changes made to the file after the mmap() call are visible in the mapped region.

about MAP_PRIVATE, I think MAP_SNAPSHOT could even be implemented just by sharpening those unspecified semantics. (However, I am not sure if MAP_PRIVATE would take a performance hit this way which would probably not be acceptable.) I guess I have to try to find someone who is able and willing to make changes to the -mm tree... :face_with_raised_eyebrow:

2 Likes

That extra bit is exactly what UnsafeCell opts out of.

Many posters on this thread are taking the Rust &[u8] guarantees too seriously :slight_smile:

It's possible for another process (with appropriate permissions) to reach in and modify any memory at runtime (this is a feature if you're a debugger), regardless of whether that bit of memory was "immutable" from Rust's pov. Turns out, everything you've been told is a lie and actually UB - and yet the sun continues to rise each morning.

In order to use shared mmapped files (C or Rust), you need to just assume some ground rules, and code appropriately for your situation - as with everything else in life. Often you just assume the mmap contents won't change because you're the only user of the file, sometimes you need advisory locking, sometimes you need anonymous mmaps, atomic read/writes, copy the data into non-shared memory before looking at it, etc, etc.

6 Likes

@gus That is essentially what we said above when we agreed that you have to put the line somewhere.

But I feel "I can break your program by opening it in a debugger and mutating its memory" is not something I would accept as a bugreport against my program. On the other hand, "I can break your program by doing cat </dev/zero >file" seems like a valid bugreport.

I think it is reasonable to expect that users will not become root and mess with /dev/mem. However, I think it is also perfectly reasonable to think that I can mutate my file system while ripgrep is running, without causing ripgrep to crash. That's why debuggers are not a concern, but mmap giving out &[u8] is.

16 Likes

Is this true? I have a library that creates process private memory maps that are only visible and accessible to the current process and that does not propagate unsafe to its API. AFAICT it is not possible to create undefined behavior by using the safe library API (e.g create two &mut to the same object, etc. even if internally there might be multiple memory addresses through which these objects are accessible).

Also, if you use memory maps in such a way that UB cannot happen, then they are safe. For example, if you create an &mut to an object in the memory map, you can take a lock (e.g. fcntl's F_GETLK), such that if any process tries to create an & or an &mut to an object in the memory map, then they have to wait. Depending on which operating system you are on, this might or might not be possible.

If you have two processes, both with a full read/write view of a memory map, then you'll need to synchronize access somehow. One can use this to synchronize the creation of &/&mut into the memory map and prevent undefined behavior.

I think the context of this conversation is filed back memory maps, so, s/if you write a Rust library that uses memory maps/if you write a Rust library that uses *file-backed* memory maps. :slight_smile:

When you create a file-backed memory map you deal with pointers. It is ok for multiple pointers to alias, and it is ok for a pointer in a different process to read to data that you are writing to through a raw pointer.

When you want to convert those raw pointers to references, and the file can be accessed by any process (read or write), then can't you just use a file lock or a record lock to either lock the whole file or lock a part of it? I mean, you can use fcntl on memory mapped files, so it is possible to create exclusive shared lock on the whole file and also memory regions within the file. You just need a "smart reference" type that on drop releases the locks. Windows must have something like this too.

Let's say ripgrep does that on every single file it searches in a directory tree. Is that going to prevent some other process---say, someone's text editor---from mutating the file at the exact moment that ripgrep decides to create a &str out of a region of memory?

Every time I've gone down this path, people have told me that file locks on Linux are advisory, and that therefore, the answer to my aforementioned question is "no." If you think differently, then I would certainly appreciate a pointer to the API that provides non-advisory file locking on Linux.

2 Likes

Yes.

If you think differently, then I would certainly appreciate a pointer to the API that provides non-advisory file locking on Linux.

The API is the same for advisory and mandatory locking (fcntl). The type of lock that a file has is a property of the file. Setting group-id bit and clearing the group-execute bit in its file mode enables mandatory locking.

This is part of the System V/POSIX API, so it should "work" modulo OS bugs pretty much everywhere (BSDs, Solaris, Linux, apple, ...). I've only used it on non-mmapped files, but an fcntl+mmap google search shows that you can use it to lock an mmapped file.

I'm not sure how you see that as a real solution to this problem. You want ripgrep to modify the file mode bits of every file it searches? Yeah, that ain't gunna fly.

I was looking for some authoritative material on the subject, since I've heard from so many people that file locking is broken on Linux. There is a fairly large hurdle in my mind to overcome. Here is some material:

  1. https://www.kernel.org/doc/Documentation/filesystems/mandatory-locking.txt (Note that bugs at the top, which would absolutely prevent me from saying that I've safely encapsulated the use of file backed memory maps.)
  2. File locking in Linux

Interestingly, (2) says that the partition needs to be mounted with the mand option, but (1) does not seem to mention that.

2 Likes

That's the only way I know of enabling mandatory file locking. If this is too slow for ripgrep, send an email to the LKML ? Maybe there is a faster way (e.g. enough people complaining that actually having to create a file to mmap something was too slow, and we got memfd_create in return).

If you have a process that wants to share some of its memory with another process through IPC, then you have to create a file anyways, and can set the files however you want (and use mandatory locking).

Have you considered advisory locking? You would need to hope that the other processes actually take the lock, but if a process does not do so then, that process is invoking undefined behavior (in C, Rust, ...).

I sound like a broken record at this point, but I do not think that this is a reasonable stanza. Normal user actions like writing to a file should not be made UB. That will not lead us to anything useful -- it will just lead us to everyone saying that all programs have UB anyway so meh. (C anyone?)

7 Likes

ripgrep could open the file by copying it to memory and call it a day. Instead it is creating a view of the file in memory, so that it can access it as shared state across all processes in the system. It cannot take a mandatory lock, because it is opening a gazillion files and that would be too slow or because they don't work properly on mmap'ed files. It doesn't take an advisory lock because if all other parties accessing the data do not respect it, then you have a data-race. So it currently takes no locks, and that might be the best call.

I think that ripgrep constraints hardly qualify as "normal user action". Trying to do what it does, properly, without losing performance, across all operating systems it targets, it's just hard or impossible to do correctly. What if the OS is buggy and the lock doesn't lock anything?

Many other applications of memory mapped files do not have the same constraints that ripgrep has, and can just lock the memory mapped file properly, or use an advisory lock and consider a process not taking the lock a data-race bug that can be fixed, or create "private" memory mapped files shared across a select set of processes that are known to respect the lock, etc.

2 Likes

Wait a minute: ripgrep doesn't use mmap when it opens "a gazillion" files, because messing around with the page tables is too slow. It specifically only uses it for single-file searches.

1 Like

I think that's immaterial to this conversation. That heuristic can change at any time and the user can override it.

1 Like

It means that "we can't use locks because it's too slow" is immaterial. mmap is already slow.

1 Like

I never said that though. Gnzlbg did. Still, though, it might make the process even slower. Moreover, that's not the only consideration involved here. There are several more that apply even to the single file use case (such as race condition bugs the make the entire enterprise moot on Linux in addition to the apparent requirement for special mount flags?).

1 Like

It seems to me that the question is whether or not it can be considered correct to have an MMAP API in Rust that returns an owned array or &mut slice that is bound to MMAP'ed memory that is not known to be "locked" against any concurrent update or read by any other process at the OS-level or a &slice that is so bound that is not known to be "locked" against any concurrent update by any other process. It seems like the answer to that question is NO because owned array and &mut slice is a guarantee at the Rust level that the underlying memory will not have concurrent updates or reads (accept in the case of "unsafe" code that is misbehaving). The same applies for &slice with the guarantee regarding concurrent writes.

Now, where there is wiggle-room to my mind is the, "accept in the case of "unsafe" code that is misbehaving" part. If another process, no matter the language, is modifying or reading memory or files through mmap or other mechanisms without honoring the appropriate protocols at the OS-level, it is "misbehaving" for some definition of that. The problem is, that a concurrent Rust program using such an MMAP API would be behaving "unsafely" all the while using an ostensibly "safe" API. This seems like a contradiction.

I would conclude, that a proper MMAP API requires the following guarantees to be considered correct with respect to Rust:

  • The API may only return an owned array or &mut slice if the underlying OS guarantees that the mapped pages cannot be read/written by any other process until the API releases the lock. Usage of the &mut would be by passing to a callback Fn. When that Fn ended, the memory lock would be released.
  • The API may olny return an &slice if the underlying OS guarantees that there will be no concurrent writers. (same thing with regard to usage via callback).
  • If the underlying OS cannot make the above guarantees, then, the only thing the MMAP may return to give access to the memory is a raw pointer. Dereffing the pointer is "unsafe" and so anything higher-level built on that usage would have to be "unsafe" as well unless it could make the appropriate guarantees through other mechanisms.

That seems correct to me. Thoughts?

2 Likes

Well, as discussed earlier in the thread, I believe it would also be safe to return an &[AtomicU8].

Edit: But also note that any use of mmap on a file fundamentally carries the risk of segfaulting the program if there’s an I/O error reading the file. Such a segfault is not inherently dangerous or memory unsafe, but it does terminate the program, which of course may be undesirable. There’s no way to prevent such segfaults, although you can potentially recover using a SIGSEGV/SIGBUS handler.

2 Likes