Re: [MirageOS-devel] Unmapping grants is unsafe

[Anil Madhavapeddy <anil@xxxxxxxxxx>]

> On 4 Jan 2016, at 14:26, Thomas Leonard <talex5@xxxxxxxxx> wrote:
> 
> On 3 January 2016 at 10:38, Thomas Leonard <talex5@xxxxxxxxx> wrote:
>> On 2 January 2016 at 14:35, Mindy <mindy@xxxxxxxxxxxxxxxxxxx> wrote:
>>> On 01/02/2016 12:14 PM, Anil Madhavapeddy wrote:
>>>> 
>>>> On 2 Jan 2016, at 12:07, Thomas Leonard <talex5@xxxxxxxxx> wrote:
>>>>> 
>>>>> I have a Mirage firewall running now under Qubes:
>>>>> 
>>>>> 
>>>>> http://roscidus.com/blog/blog/2016/01/01/a-unikernel-firewall-for-qubesos/
> [...]
>>>>> - Mirage allows you to access memory after unmapping it. This isn't
>>>>> safe (may page-fault or give access to someone else's data). I've
>>>>> added checks in mirage-net-xen to make sure we don't try to access a
>>>>> ring after unmapping it [1], but I think this should be fixed at the
>>>>> Bigarray level [2].
>>>>> 
>>>> Did that Bigarray patch get committed into OCaml 4.03dev?  I dont
>>>> have a checkout handy here and it's hard to map a commit to the
>>>> associated PRs in the web UI.
>> 
>> I haven't submitted a PR yet. Need to update it to master and test.
> 
> https://github.com/ocaml/ocaml/pull/389
> 
> However, they're not keen on merging it, for performance reasons.
> Also, it would mean changing Cstruct, which currently says:
> 
>> We try to maintain the property that no constructed [t] can ever point out of
>> its underlying buffer. This property is guarded by all of the constructing
>> functions and the fact that the type is private, and used by various
>> functions that would otherwise be completely unsafe.
> 
> I wonder if we should instead change Gnt.Gnttab.Local_mapping.to_buf
> so that instead of returning an Io_page it returns something else,
> which provides similar operations but:
> 
> a) checks every access to ensure it hasn't been unmapped, and
> b) doesn't allow you to get a Cstruct or Bigarray from it.
> 
> However, this would mean a largish API change.

Before going through the epic amount of work that an API change would make in 
upstream libraries, perhaps we should build a small unit test that rapidly does 
a simple data copy across domains and benchmark it?

I think any new API should do very explicit lifetimes for the grant buffers 
(i.e. not depend on the GC to cleanup), but also minimise copying the data if 
at all possible.  While it's dangerous to read from a shared memory buffer if 
you don't trust the other side, an enforced copy does make it hard to build 
efficient unikernel proxies that are just transferring data across shared rings.

Anil


_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel