Would it be though? Would other processes be accessing the memory through the atomics protocol? I would think not necessarily. Or, am I missing something?
At least for this discussion, I think it is important to distinguish the use cases of mmap, i.e. using it to share memory between cooperating processes essentially using the file system as a common namespace or what I am more interested here (and what ripgrep needs): Using mmap as a simple method to do optimized parallel I/O via the kernel's page cache and read-ahead mechanisms but with a user-space API that is as simple as it gets, i.e. a flat region of memory. In applications of this like searching, indexing or general parsing, I usually also cannot assume any form a of cooperation, e.g. no advisory locks, and I generally could not even block other processes for reasons of usability.
Hence, I'd really to have a version of mmap that is able to support an API that yields &[u8] without any external guarantees beyond that integrity the kernel provides for my process anyways, i.e. using /proc/$pid/mem is as irrelevant as advisory locking. And since MAP_PRIVATE basically provides the converse of the necessary guarantees, I think we should actually try to something like MAP_SNAPSHOT from the kernel which would make this possible and be a nice example of coevolution between programming languages and OS kernels, something that C/C++ has excelled at in the past.
1 Like
I think if you want to reliably communicate with that process, then both sides should use atomic access. But if the other process only "interferes" with your read access, then only you using them should be fine since they basically say that this memory can be changed by another process and you need to explicit when ask the memory subsystem for the values stored therein.
But then again, &[AtomicU8] does not look like something that would benefit usage via nom or regex which is why I keep saying that we really want to have &[u8] even if it means that the kernel has to be extended to accommodate memory safety instead of changing Rust's mmap API to match the limited POSIX semantics.
2 Likes
Awe, yes. I see your point now. That does make sense. I agree 100%.
EDIT: I wonder how much this discussion, Two Kinds of Invariants: Safety and Validity - #18 by RalfJung - Unsafe Code Guidelines - Rust Internals, impinges on this notion of MMAP being safe vs. unsafe.
In particular, I think that the notion of "validity" is even a bigger issue for MMAP.
I guess if we talk about how a safe mmap API could look like, then safety is the main issue. After all, an mmap API could just bail out and yield *u8 and leave everything up to its user. But yes, if one looks at the current API of the memmap crate and does feel uncomfortable leaving the necessary unsafe block, then one is left with unsafe access to a &[u8] that might even be invalid in the sense of the linked discussion/blog post. But I have to admit that after reading @RalfJung's blog post, I am still not sure whether e.g. immutability of referenced data is part of validity if even initialization is not...
Does the sum of these concerns explain or offer a good reason why the memmap crate has gone dormant for 8 months? I'm thinking of forking it for huge page (e.g. HUGE_TLB | HUGE_TLB_2MB) support, which has gone unfinished in its README roadmap for three years.
It is not part of validity -- but it is part of safety, which is the more relevant invariant for a safe API.
On Windows you can get EXCEPTION_IN_PAGE_ERROR exceptions but they're not UB or dangerous, you just have to use SEH exception handling to catch the exception and deal with it. If only Rust actually had SEH exception handling...
There's nothing special about mmap calls used "late" in program execution. The actual executable text (and libraries) are also mmapped, and just running normal code can trigger a SIGBUS at any point due to I/O errors.
Likewise, someone can mmap the executable file and start modifying it's contents just as easily as they can do that to any other file. I hope it's sufficiently absurd to consider marking all the compiled Rust code as unsafe (or fence every instruction with atomic barriers).
And: Mandatory locking fails on many network filesystems.
I'll say again: we're taking it all too seriously. In practice the problems described here never occur because everyone knows that modifying the contents of a file in place will upset mmap users, and so they avoid doing it where they expect to see mmap usage. (eg: editors usually write new files and rename into place, and nobody modifies the bytes in the middle of their DB backing file without preparation.) If a user violates this arrangement and does pathologically modify a file while simultaneously running ripgrep, ripgrep will just crash. It's not that bad.
TL;DR: I feel the various solutions offered in this thread are significantly worse than the original problem. I think the last time I actually tripped over the original concern was back in 1990's - I untarred a new libc over my existing libc and hung just about all existing processes - and tar has been modified since to write new files.
4 Likes
But there is something special about the ability to change the files backing running programs, i.e. you are the systems administrator or put differently: There is a reason a normal user account is usually not allowed to modify /usr/lib/libc.so, because it could lead to crashes and undefined behavior in almost every running process. (If you run a binary that you can change, it is your responsibility not to change it while it is running.)
I guess it depends on what your definition of bad is, but if it were just about "clean" crashes via SIGSEGV or SIGBUS, we wouldn't need to have this discussion. But Rust (and hence rustc and hence LLVM) assumes that &[u8] is not modified concurrently and hence you do not necessarily get a crash but are faced with the problem of undefined behavior and thereby an exploitable security issue in the worst case.
6 Likes
.. and this is exactly the point I'm trying to get to. The whole thing just trusts the user not to mess with mmapped files, and it's ok! 
It's starting to feel like to me that if we can pin down whether or not mmap can ever be called "safe" we will have sufficiently defined "safe" for all purposes.
I've been trying to get people to consider that the extreme/theoretical question ("can mmap ever be called safe?") is not useful 
The answer is easy: "no". Indeed, all memory can be modified, even the program text, and it can be modified as easily/carelessly as modifying the backing store of an mmapped data file.
The question (imo), is really "what abstraction usefully represents the situation, within the bounds of the usage we expect to encounter?"
The answer to this is: "depends".
If you expect a file isn't going to change in-place "magically", then &[Copy] (or &mut [Copy] for read/write mmaps) is fine for all real-world cases, and will enable the compiler to do lots of sensible and reasonable optimisations. This matches how "normal" programs use mmap on Unix.
If you expect your mmapped file is going to be changed by other processes (anonymous mmap shared amongst forked children, DB backends working on a shared mmapped data file, etc) then you'll need to add some cooperative locking, or restrict yourself to atomic operations, and flush appropriately. Again, this turns into &mut [Copy] with a lock guard, or &mut [AtomicSomething] in an exposed Rust API.
If you need to code defensively because you expect to be attacked, or your underlying device to drop out, then you're doomed in the full general case. For the limited case where you are considering that threat via a particular mmapped file operation only, then you can do expensive things like copy data into in-process memory before interpreting it (eg: kernel does this for sensitive user-space memory), or limit yourself to atomic operations and careful logic, and you'll probably need a custom SIGBUS/SIGSEGV handler to recover from missing pages. Honestly, I would strongly recommend skipping mmap and use regular file I/O for this case, unless you have no choice but to deal with memory-mapped data (eg: you're writing a kernel or other sandbox).
Again: a theoretical analysis in isolation of how all this really works underneath is not useful, and just leads to marking everything as unsafe for everybody. Turns out, it's all literally built on sand 
I don't know what this means for a Rust API, since we can't just make a useful blanket statement. Given the patterns of usage in the wild, I suggest we just provide the first case (assume you have exclusive access to the file and return naive &[Copy]), with documentation warnings that this is not sufficient for sophisticated mmap users. I understand that this ambiguity will be upsetting to people who came to Rust desiring guarantees that Unix does not actually provide.
(All imo, of course)
3 Likes
I tend to disagree, since Unix is setup so that the majority of binaries cannot be changed while I am executing them.
Then the guarantees that Unix provides going to have to change.
Honestly, this is not about whining that things are messy and complex and that memory safety is no panacea either, but if you noticie that some functionality like memory-mapped I/O has serious issues together with some other functionality like Rust's memory safety features, you try to take a whole-system POV and consider how to fix the mismatch between the parts that do not work well together.
And Rust tries significantly harder to drill down into that than let's say C/C++ (instead of fleeing upwards like e.g. Java/.NET) and tries to find (or make) the places that stick so we actually get out of the "in the end, every non-trivial program has UB, it just doesn't happen" mess systems programming is currently in.
5 Likes
I agree with others that have pointed out that these two things aren't the same class of danger. On the surface they are, but, in reality, the danger of arbitrary files that you might mmap being concurrently modified is highly more likely and possible than the chance of libs and binary executables being modified whilst running. They just aren't the same danger in reality due to the nature of default file-system privileges, layout, expectations,
and conventions.
This is, I think, is the important thing to consider: What are the expectations that the type returned by the API convey and are those expectations "guaranteed" (for some definition of guaranteed) to be be upheld. As long as the "guarantees" match the expectations I believe we could consider the API "safe". If the guarantees don't really match the expectations, then the API should be considered "unsafe" to my mind.
I believe that would be akin to throwing in the towel. It seems like there might be a better path that can reasoned through here.
Yes! This! 100% This!
To me this thinking leads to:
- MMAP API without page-locking guarantees MUST be "unsafe" and only return a raw-pointer to the mmap'ed pages (or some kind of new "unsafe" slice)
- MMAP API with properly enforced page locking by the OS can return a "safe" slice (mutable or not depending on the type of lock)
- If Linux (or any other OS has broken locking), then on that OS the Safe API is not available. If we want the Safe API there, then we need to get the Kernel to fix the issue first. NOTE: It's up for debate whether mandatory vs. advisory locking can be considered safe, but, It seems like leaning towards mandatory would be the more prudent path.
That to me seems like the only reasonable and prudent way forward.
2 Likes
I understand I'm no longer adding anything useful to this conversation, so I'll bow out here, but I am confused at the inconsistency in these statements (and similar comments above, not just this/yours). In the former you're willing to rely on loose conventions and the "likelihood" of files being modified, whereas in the latter you're demanding nothing less than strict mandatory page locking.
As far as I can see, the only difference is whether you can "see" the mmap function call in your code, but I assure you it's there in both cases. Applying an argument centred around theoretical strictness to one case but not the other seems a bit arbitrary.
As previously pointed out by @adamreichold:
There is something entirely different about these things from the perspective of the exposure to non-Admin maliciousness or carelessness that makes these things not in the same class. It's not the case that those of us arguing for a stricter definition around the safety of MMAP are arguing for loosey-goosey in one case and strict theoretical consistency in the other for 2 cases that are the same. They aren't the same as has been pointed out by several individuals. You may think they are the same, and you are surely entitled to your opinion, but, many think they are not the same. Arguments for/against why they are/are not the same are welcome. Declaring them the same and then saying that having different treatments is not appropriate because they are the same is a logical fallacy.
Only if the app is installed through a system package manager. If I install rustc through rustup, it's actually stored in ~/.rustup, and can be modified at will without being the system administrator. Though, honestly, I think we can throw in the towel at that point, just like we can't do anything about /proc/self/mem. If you told somebody that modifying the app while it's running could trigger undefined behavior, they probably wouldn't be surprised.
mmap itself should be marked unsafe. Unlike modifying a running binary, where most developers would immediately recognize it as a problem, mmap seems like it could conceivably be a safe interface, but it's not. Rust should mark mmap as unsafe, because it really is, and any developers who want to use it should write the unsafe block and Deal With the fact that their program exhibits undefined behavior if the file is modified underneath it.
1 Like
When I do cat foo > binary while binary is running (and I have write permissions for that file), I get "Text file busy". I can do rm binary; cat foo > binary but that does something very different; the original binary file is preserved in this case and only deleted when the program quits. So it does not seem easy for a user to change the content of a running binary.
Does anything similar happen for files mmap'd, for example, by ripgrep?
4 Likes
Slow down, everyone!
I believe Rust should properly respect the semantics of mmap, including in situations where mandatory locks are unavailable. That does not mean it has to be unsafe.
To summarize, mmap has two main risks:
1. I/O errors can cause faults, aborting the program
Like I said, this can be a problem, but it's not unsafe, any more than aborting the program by double-panicking or overflowing the stack is unsafe. How much of a problem it is depends on the use case.
For a command line tool like ripgrep, where each user 'query' is done with a separate invocation of the executable, it's not a huge deal if it crashes – although it would still be nicer if it cleanly reported the error. For a server program which handles many queries, in theory it could be more problematic. On the other hand, most server programs are not designed to operate on storage that might be unreliable; that's simply out of their scope, the same way that I/O errors reading the program itself are necessarily out of scope. With grep, by contrast, I often end up searching for certain terms through large directory trees, sometimes even the entire filesystem – and you never know if there's some broken network mount sitting around somewhere, or a USB drive that gets yanked out halfway through.
But in theory ripgrep could handle that case by installing a signal handler, and in any case it's out of scope of a discussion about unsafety.
2. Memory can be changed out from under you
So we can't return &[u8]. But that's okay. We can return &[AtomicU8], and design APIs to accept both types of buffers whenever possible.
Accessing an element of an &[AtomicU8] is not necessarily more expensive than accessing an element of an &[u8]; if you use a relaxed load, it will usually produce identical assembly.
At the moment it seems that LLVM will coalesce multiple loads into one with regular loads but not atomics… but there's no theoretical reason it couldn't perform the same optimization for both, and in any case the generated assembly is rather suboptimal in both cases (doing redundant length checks).
More optimized implementations typically don't read bytes one-by-one, but use alternate strategies to access data, generally based on unsafe code. Such as:
- Casting to other pointer types before reading: Just as
&[u8]can be safely converted to, say,&[u32](if correctly aligned),&[AtomicU8]can be safely converted to&[AtomicU32]. - Using
memcpy: It should be fine to write a wrapper formemcpythat takes&[AtomicU8]as a source and/or destination, just as in C it's OK to usememcpyon a buffer that might be concurrently mutated. - Optimized assembly or SIMD intrinsics: Also should generally be safe to use with
&[AtomicU8].
This is not only useful for mmap. It can also be useful for buffers that are mutated by other parts of the same program. In one Rust program I wrote, I had a large buffer representing an executable file. At first the program was only designed to parse the executable format, which involved saving slices to various portions of the buffer; originally these were &[u8]. However, it turned out that for certain functionality, I also wanted to patch the executable, i.e. modify the buffer. Furthermore, for maximum speed, I wanted to do this in-place rather than making any copies. So I decided to transition everything to &[AtomicU8]. This wasn't as disruptive as it sounds, because most of my reads were based on memcpy anyway; I just wrote a "buffer" trait that's implemented for both &[u8] and &[AtomicU8], so my (custom) wrapper functions would work on both.
3 Likes