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

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

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