Document Rust's stance on /proc/self/mem

[sunfishcode]

Add documentation to std::os::unix::io describing Rust's stance on
/proc/self/mem, treating it as an external entity which is outside
the scope of Rust's safety guarantees.

[rust-highfive]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[joshtriplett]

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @cuviper
• @joshtriplett
• @m-ou-se
• @yaahc

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[JakobDegen]

@sunfishcode that seems like a reasonable set of rules. It's a little unclear to me how dlclose fits into that, but if you can argue that it does, I'd be happy.

But is that set of rules what this FCP is approving? Seems unlikely, since it wasn't even written down before now. Maybe it was just unclear to me though.

[sunfishcode]

@sunfishcode that seems like a reasonable set of rules. It's a little unclear to me how dlclose fits into that, but if you can argue that it does, I'd be happy.

Are you asking why we say that resources accessed through the filesystem namespace are safe, when resources accessed through the dynamic linking namespace are unsafe?

In the filesystem case, it's because filesystem APIs are widely used, and most of the things that most people do with the filesystem namespace don't cause UB most of the time. Making it unsafe would be coherent, but on a practical level, it would likely do more harm than good. Making it safe is also coherent, using of this internal/external dichotomy, and much more practical, so we do that.

In the dynamic linking case, the things the dynamic linker produces are raw pointers. The thing most people do with them most of the time is call them, and if calling them could invoke UB, we have to treat them as unsafe. They're internal in the dichotomy—they're loaded into the program. Encapsulating this unsafety requires knowing the conditions under which they'll invoke UB, and the only way to know that is to know and trust the library they're loaded from, and have a commitment from that library that they'll behave. With that framework in mind: dlclose is loaded from this library called "libc" which we probably know and trust, but libc documents that dlclose in particular does multiple things which are quite unsafe, in a Rust sense. Consequently, dlclose is unsafe.

But is that set of rules what this FCP is approving? Seems unlikely, since it wasn't even written down before now. Maybe it was just unclear to me though.

The proposed change contains a lot fewer words, but I believe it's saying essentially the same thing.

Rust isn't alone here; lots of programming languages have effectively the exact same rules, yet as far as I know, none of them document the rules as such. I think there's a widespread assumption that this overall framework is just how computers are understood to work and it doesn't need to be stated.

That may mean we have a curse-of-knowledge phenomenon going on. Once one has the intuition here, there seems to be a sense of inevitability about the rules. /proc/self/mem is a little surprising at first, but once one discovers the solution of classifying it as external, it lines up with the intuition and feels similarly inevitable.

So perhaps what we're seeing here and in other places where people keep asking questions about this, is that not everyone makes the same assumptions or thinks about this the same way. In that case, perhaps the path forward looks like:

• Assuming this FCP completes here, we merge this PR, establishing that this is T-libs' official position, at least in the eyes of people who share the overall intuition.
• Then, someone could write up the rules above in a form that could be added to the language-level documentation, perhaps in the Nomicon, because I think this relates to your questions about why this isn't a T-lang/UCG question.
• Then, we could update the library documentation here to reference that language documentation.

How does that sound?

[ChrisDenton]

Making it unsafe would be coherent, but on a practical level, it would likely do more harm than good.

This I very much agree with. Rust's stance can be fully justified on pragmatic grounds.

What I'm less clear is on is the proposed "internal" and "external" dichotomy. If I understand you right, a hypothetical libc::corrupt_my_memory function would be internal but a File::open("/special/corrupt_my_memory") would be external? Even if they used the same underlying mechanism?

Also, if Rust could encapsulate "person with a soldering iron" then surely it should do so? The problem with that is a practical one, not a philosophical objection to doing so.

[sunfishcode]

What I'm less clear is on is the proposed "internal" and "external" dichotomy. If I understand you right, a hypothetical libc::corrupt_my_memory function would be internal but a File::open("/special/corrupt_my_memory") would be external? Even if they used the same underlying mechanism?

The underlying mechanism matters. If it's doing File::open, users have the option to control what it can do by running it in a sandbox.

Also, if Rust could encapsulate "person with a soldering iron" then surely it should do so? The problem with that is a practical one, not a philosophical objection to doing so.

"Is there a boundary?" is philosophical while "where is the boundary?" is practical.

[cuviper]

a hypothetical libc::corrupt_my_memory function

There is libc::process_vm_writev, but at least that's unsafe, like any FFI. However, nix has a safe wrapper, which is probably a mistake.

[sunfishcode]

In theory, process_vm_writev could go either way. On one hand, users could stop it with seccomp or by withholding PTRACE_MODE_ATTACH_REALCREDS, so one could reasonably classify the access as "external" and call it safe. On the other hand, it's not widely used, and it's quite easy to get it to invoke UB in the calling process, so it seems practical and more desirable to classify it as unsafe.

Edit: Also, unlike /proc/self/mem, it appears process_vm_writev bypasses prctl(PR_SET_DUMPABLE, SUID_DUMP_DISABLE), which is a pretty strong signal that it doesn't behave like a "debugger" feature, and thus is very desirable to classify as unsafe.

Edit: It also bypasses /proc/sys/kernel/yama/ptrace_scope even in its most restrictive setting 3, no attach.

Edit: I was testing calling process_vm_writev on the same process. Linux's ptrace access rules start with "If the calling thread and the target thread are in the same thread group, access is always allowed."

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[m-ou-se]

@bors r+

[bors]

📌 Commit 6959441 has been approved by m-ou-se

[JakobDegen]

I unfortunately didn't get a chance to respond before, and not sure if it's worth doing it now, but I guess I will anyway.

Rust isn't alone here; lots of programming languages have effectively the exact same rules, yet as far as I know, none of them document the rules as such. I think there's a widespread assumption that this overall framework is just how computers are understood to work and it doesn't need to be stated.

Not at all. There's no other language I know of that attempts to make a formal distinction between "sound" and "unsound," and so no other language has to deal with this issue. I think I've actually made a mistake in not emphasizing this point enough in this conversation. "Sound" and "unsound" are not (supposed to be) vague ideas that represent some philosophical goals, they are concrete concepts that we hope to tie to a formal definition. I don't mean to be rude here, but it frankly seems like this is being hand-waved away both in this conversation and in the PR itself, and I think that is a distinctly dangerous road to go down.

I went back and re-read your proposed rules, and honestly I'm not quite sure why I was happy with them the first time around - I don't agree with the distinctions, and I especially don't believe that arguing these kinds of points from a list of examples is a way forward. I have some alternative ideas in mind, and would be happy to discuss more on Zulip instead of here.

So perhaps what we're seeing here and in other places where people keep asking questions about this, is that not everyone makes the same assumptions or thinks about this the same way. In that case, perhaps the path forward looks like:

• Assuming this FCP completes here, we merge this PR, establishing that this is T-libs' official position, at least in the eyes of people who share the overall intuition.
• Then, someone could write up the rules above in a form that could be added to the language-level documentation, perhaps in the Nomicon, because I think this relates to your questions about why this isn't a T-lang/UCG question.
• Then, we could update the library documentation here to reference that language documentation.

How does that sound?

Honestly, about as backwards as could be. "Stabilize that this API is sound first and figure out some rules under which that makes sense later" is not at all how these things should work. When we add the language level documentation, what choice will be left for T-Lang and T-types? What if they (like me) don't agree that we can just call this sound? It's not like they'll be able to say no anymore.

I suppose it's on me that I failed to ask for this to be nominated for T-Types earlier. I guess it's too late now?

[ChrisDenton]

I mean, it's not set in stone until it reaches stable? I think you can still discuss this on Zulip and ask t-libs if you have reasons they might want to reconsider.

I personally don't want to take issue with /proc/self/mem being safe (even if it's far from ideal). I am however uncomfortable with the broad reasoning given in this PR, particularly how it might be applied to other cases that can be encapsulated without the same practical problems of making File unsafe to use.

[JakobDegen]

Yeah, agreed. If this was just documenting that std doesn't care about protecting you from /proc/self/mem we'd be fine.

That's a good point about us having until this reaches stable. I might bring this up with T-types on zulip then (as they are the proper team to be making this decision). Later though, it is bed time for me right now

[SimonSapin]

For anyone else looking, this doc-comment ends up visible at: https://doc.rust-lang.org/nightly/std/os/unix/io/index.html#procselfmem-and-similar-os-features