Mailing List Archive: [PATCH 19/23] arch/x86: check remote MMIO remap permissions

[PATCH 19/23] arch/x86: check remote MMIO remap permissions

Sep 17, 2012, 8:23 AM

Post #1 of 3 (154 views)

When a domain is mapping pages from a different pg_owner domain, the
iomem_access checks are currently only applied to the pg_owner domain,
potentially allowing the current domain to bypass its more restrictive
iomem_access policy using another domain that it has access to.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>
---
xen/arch/x86/mm.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5c6ea2d..c0c2bf3 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -754,6 +754,18 @@ get_page_from_l1e(
return -EINVAL;
}

+ if ( pg_owner != curr->domain &&
+ !iomem_access_permitted(curr->domain, mfn, mfn) )
+ {
+ if ( mfn != (PADDR_MASK >> PAGE_SHIFT) ) /* INVALID_MFN? */
+ {
+ MEM_LOG("Domain %u attempted to map I/O space %08lx in domain %u",
+ curr->domain->domain_id, mfn, pg_owner->domain_id);
+ return -EPERM;
+ }
+ return -EINVAL;
+ }
+
if ( !(l1f & _PAGE_RW) ||
!rangeset_contains_singleton(mmio_ro_ranges, mfn) )
return 0;
--
1.7.11.4

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 19/23] arch/x86: check remote MMIO remap permissions [ In reply to ]

JBeulich at suse

Sep 18, 2012, 12:21 AM

Post #2 of 3 (148 views)

Permalink

>>> On 17.09.12 at 17:23, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -754,6 +754,18 @@ get_page_from_l1e(
> return -EINVAL;
> }
>
> + if ( pg_owner != curr->domain &&
> + !iomem_access_permitted(curr->domain, mfn, mfn) )

On a second (or third?) look, I don't think this is right - the current
domain doesn't matter here at all. What does matter is who's page
tables the mapping is to be put in (i.e. l1e_owner), which of course
in certain cases ends up being the current domain.

Jan

> + {
> + if ( mfn != (PADDR_MASK >> PAGE_SHIFT) ) /* INVALID_MFN? */
> + {
> + MEM_LOG("Domain %u attempted to map I/O space %08lx in domain %u",
> + curr->domain->domain_id, mfn, pg_owner->domain_id);
> + return -EPERM;
> + }
> + return -EINVAL;
> + }
> +
> if ( !(l1f & _PAGE_RW) ||
> !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
> return 0;
> --
> 1.7.11.4

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 19/23] arch/x86: check remote MMIO remap permissions [ In reply to ]

dgdegra at tycho

Sep 18, 2012, 7:09 AM

Post #3 of 3 (143 views)

Permalink

On 09/18/2012 03:21 AM, Jan Beulich wrote:
>>>> On 17.09.12 at 17:23, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -754,6 +754,18 @@ get_page_from_l1e(
>> return -EINVAL;
>> }
>>
>> + if ( pg_owner != curr->domain &&
>> + !iomem_access_permitted(curr->domain, mfn, mfn) )
>
> On a second (or third?) look, I don't think this is right - the current
> domain doesn't matter here at all. What does matter is who's page
> tables the mapping is to be put in (i.e. l1e_owner), which of course
> in certain cases ends up being the current domain.
>
> Jan

Ignoring current->domain allows a malicious domain with access to
remotely manipulate page tables to create a device mapping where the
target domain doesn't expect one. Except, now that I've said that ...
any domain with the ability to change another domain's page tables
almost certainly can control execution in that target domain, so that
attack doesn't get you anywhere you couldn't already go.

Checking the page table owner for iomem access is useful here, I'll
change the patch to do that.

>> + {
>> + if ( mfn != (PADDR_MASK >> PAGE_SHIFT) ) /* INVALID_MFN? */
>> + {
>> + MEM_LOG("Domain %u attempted to map I/O space %08lx in domain %u",
>> + curr->domain->domain_id, mfn, pg_owner->domain_id);
>> + return -EPERM;
>> + }
>> + return -EINVAL;
>> + }
>> +
>> if ( !(l1f & _PAGE_RW) ||
>> !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
>> return 0;
>> --
>> 1.7.11.4

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel