From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Subject: Re: [PATCH 19/23] arch/x86: check remote MMIO remap
	permissions
Date: Tue, 18 Sep 2012 10:09:08 -0400
Message-ID: <50588084.3090300@tycho.nsa.gov>
References: <1347895425-17959-1-git-send-email-dgdegra@tycho.nsa.gov>
	<1347895425-17959-20-git-send-email-dgdegra@tycho.nsa.gov>
	<50583D35020000780009BFB0@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <50583D35020000780009BFB0@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>, keir@xen.org, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

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