Open asajeffrey opened 4 years ago
The equivalent trait in shared-data
is SharedMemRef
, described in https://docs.rs/shared-data/0.0.1/shared_data/trait.SharedMemRef.html but I think I'm dealing with a different attacker model, in particular allowing attackers to write to shared memory (but not truncate it, c.f. https://github.com/asajeffrey/shared-data/issues/7). This means that &usize
isn't safe to take into shared memory, as Rust assumes that the data pointed to by &usize
is immutable.
We did not consider the problem of peers being malignant when working on our shared memory use-cases as all the code we cared about was controlled by us in its entirety.
The traits in this crate presume that mutability properties of certain data are maintained and so &T
would not change from under your feet if T has no interior mutability. This being shared memory, one cannot really take a &mut T
either. This means that T
s for most practical purposes end up containing interior mutability anyway, usually in form of Atomic*
types, and bare usize
explicitly models some sort of immutable data that no peer should modify. In application code this might be modeled by marking certain pages of memory read-only.
That being said, my memory of this crate is fairly dry by this point, @Ekleog or @lovesegfault might have something else to add.
IIRC we used to make it so that readers had RO access, cc. @Ekleog.
Also, @nagisa, we should really open source shmem, I'll do it while sick.
EDIT: Leo did it! https://github.com/standard-ai/shmem @asajeffrey ^
IIRC we used to make it so that readers had RO access
That is no longer true, and doesn’t really solve any of the issues raised by the OP in the first place AFAICT, as writer would still be able to invalidate compiler’s notion of &T
being immutable by concurrent writing.
Yeah, absolutely. I think if your writers are evil than you're always going to be fucked, no?
Well shared-data tries to be safe even in the presence of malicious writes; malicious truncation of the shared memory file is something you can't do much about. https://github.com/rust-lang/unsafe-code-guidelines/issues/215 has a lot of discussion about what Rust can assume in terms of its run-time environement, and https://github.com/rust-lang/unsafe-code-guidelines/issues/152 is a discussion of volatile read of untrusted memory (the issue there being that LLVM goes and treats that as potentially being UB, and Rust inherits that from LLVM, sigh).
There are some types that are safe to take references to, even in the presence of malicious writers. The poster child is &AtomicUsize
which is safe to use no matter what writes the attacker does (OK, as long as you restrict the attacker so they can't write poison).
@asajeffrey In this crate, we are considering all the processes having access to the shared memory region as being bound by the rust type system, and as respecting the invariants in the unsafe functions that can be called -- in this way, they are assumed to be adverse actors but bound by the type system and invariants of the functions.
The aim is to allow for fast cross-process communication, for which volatile read/writes would be impossible.
Also, volatile read/writes are not enough to prevent all UB, and you would actually need de/serializing to protect the stuff properly -- for instance, as far as I understand your code, Volatile<NonZero>
is a legitimate type, and could actually evaluate to zero against a malicious opponent, which is UB.
That being said, I think I would see a point in having a trait for “this type is safe to put in shared memory in a purely adversarial context (ie. the opponent has full memory access) and to take a reference to it,” but I am not sure it could reasonably apply to anything due to the truncation issue. Do you have cases where you're 100% sure to be doing the right thing? Even &AtomicUsize
might see its file truncated, which would end up in a SIGBUS, I think -- and as far as I know this is not a valid behavior for safe code, though I guess that's debatable?
As for where the traits should live, to be honest, I don't really think the most basic traits to interprocess communication should have any dependency or any code besides the most minimal one, as they ideally would end up in libstd
at some point -- hence our creating a separate library for just this purpose.
Yes, our use-case is slightly different, it's the inter-process communication in Servo, where different processes correspond to different security domains. Ideally even if an attacker compromised one of the security domains, they wouldn't be able to compromise the others, which is why we're worried about attackers being able to write into shared memory. We're also concerned about attackers being able to truncate shared memory, but there's not much we can do about that on non-Linux OSs.
NonZero
doesn't implement SharedMemCast
or SharedMemSync
so Volatile<NonZero>
isn't a useful type.
Truncation is a real pain, ignoring it there are perfectly good types that are SharedMemCast
, e.g. usize
. Essentially SharedMemCast
is types where every bitstring of the right length is a valid value, and SharedMemSync
is the same, but with atomic access.
Looks like I had misunderstood your code, then, thank you for the correction.
I guess the biggest issue I am having with SharedMemCast
and SharedMemSync
is that I can't really see them being implemented wider than a few built-in types, as any kind of struct
or enum
without repr
already potentially embeds padding and thus is not either of those traits. (BTW, I see impl<T1: SharedMemRef> SharedMemRef for (T1,)
, are tuples' memory representation stable yet? I thought not).
As such, I'd expect any communication with an adversary to happen over a channel that transmits raw bytes, and to happen with serializing and deserializing, so that errors are handled properly anyway -- having the channel be implemented with volatile writes/reads on memory would then just be an implementation detail.
In case that's the way you want to go, there's a crate we should be open-sourcing soon, that provides a ringbuffer in shared memory. Its threat model was also designed with cooperative processes in mind, but off the top of my head it should not lead to UB with an adversary -- assuming a few additions to the API are made, to return a pointer instead of a reference at a few places, and to allow pushing multiple values at once for optimization purposes if you're going the RingBuffer<u8>
way (as we are doing cooperative processes, we just push bigger structures, so that's not an immediate requirement for us).
@Ekleog For structs, I was hoping to be able to derive SharedMemSync
in the same way as for SharedMemCast
. I don't think Rust requires padding to be immutable, so something like (AtomicU8, AtomicUsize)
should be fine, even though the padding is in volatile memory. I guess the unsafe code guidelines team might have something to say, I'll post a comment on https://github.com/rust-lang/unsafe-code-guidelines/issues/152.
For enums I wote some notes at https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=646c26558c0844392716497ba6c549e6, the rough idea being to only support #[repr(C,u8)]
enums and even then using a limited API.
Part of the aim here is to avoid the serialization/deserialization that ipc-channel currently uses, and provide zero-copy ipc channels. Servo currently uses shared memory for byte arrays as a special case to move images around without copying, it would be nice if that wasn't a special case any more.
Your ring buffer looks really interesting, you can see the first shot I made at channels at https://github.com/asajeffrey/shared-data/blob/master/src/shared_channel.rs, the main difference AFAICT is that Servo needs unbounded channels (our deadlock-freedom properties depend on that).
@asajeffrey What about passing types marked by the trait StableABI? The lib is a little on the heavy side, but essentially you can #[derive(StableAbi)]
for anything that is repr(C)
or a primitive. This would allows you to directly pass types across processes without needing to serialize.
It's not obvious what the safety properties of
SharedSync
are expected to be. In particular, ifT
isSharedSync
, is it safe to take an&T
into shared memory? If so, what is the attacker model, can the attacker write into the shared memory? Can they truncate the shared memory?