[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MirageOS-devel] Unmapping grants is unsafe

  • To: Thomas Leonard <talex5@xxxxxxxxx>
  • From: Anil Madhavapeddy <anil@xxxxxxxxxx>
  • Date: Tue, 19 Jan 2016 22:04:07 +0000
  • Cc: "mirageos-devel@xxxxxxxxxxxxxxxxxxxx" <mirageos-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 20 Jan 2016 03:26:15 +0000
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=recoil.org; h=content-type :mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; q=dns; s= selector1; b=Y01gRbiY4pzBv/Fvl6DGwZ3FO90MpmJyCedmUZARrWKdBKWbwa0 SEnMGQ9FqUeB75Ei9mkGr61vHF/aj2Xpon1/ey9T8u5NSwO0zIJkzs3ko7vpY6oM Yr8jqsca3enJCp9mMcuDGre/57s8pOxvyp83rivc0HOf0lL7VL3jFJmg=
  • List-id: Developer list for MirageOS <mirageos-devel.lists.xenproject.org>

> On 17 Jan 2016, at 14:54, Thomas Leonard <talex5@xxxxxxxxx> wrote:
> On 15 January 2016 at 16:15, Anil Madhavapeddy <anil@xxxxxxxxxx> wrote:
>>> 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.
> A simpler solution might be to rename [to_buf] to [to_buf_unsafe] and
> provide a safe [blit] operation (that fails if the buffer is no longer
> mapped). The block and net drivers already copy, so they could use the
> safe interface.
> shared-memory-ring currently reads directly, but maybe it shouldn't
> anyway (whether frontend or backend) to avoid TOCTOU issues.

Yes, this sounds reasonable for the block and net drivers, and any lingering 
users of `to_buf` could be fixed up easily.

MirageOS-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.