From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Subject: Re: [PATCH 17/20] arch/x86: use XSM hooks for
 get_pg_owner access checks
Date: Tue, 11 Sep 2012 09:40:43 -0400
Message-ID: <504F3F5B.6070200@tycho.nsa.gov>
References: <1347306553-20980-1-git-send-email-dgdegra@tycho.nsa.gov>
	<1347306553-20980-18-git-send-email-dgdegra@tycho.nsa.gov>
	<504F0AA9020000780009A704@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: <504F0AA9020000780009A704@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 Fraser <keir@xen.org>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On 09/11/2012 03:55 AM, Jan Beulich wrote:
>>>> On 10.09.12 at 21:49, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -2882,11 +2882,6 @@ static struct domain *get_pg_owner(domid_t domid)
>>          pg_owner = rcu_lock_domain(dom_io);
>>          break;
>>      case DOMID_XEN:
>> -        if ( !IS_PRIV(curr) )
>> -        {
>> -            MEM_LOG("Cannot set foreign dom");
>> -            break;
>> -        }
>>          pg_owner = rcu_lock_domain(dom_xen);
>>          break;
>>      default:
>> @@ -2895,12 +2890,6 @@ static struct domain *get_pg_owner(domid_t domid)
>>              MEM_LOG("Unknown domain '%u'", domid);
>>              break;
>>          }
>> -        if ( !IS_PRIV_FOR(curr, pg_owner) )
>> -        {
>> -            MEM_LOG("Cannot set foreign dom");
>> -            rcu_unlock_domain(pg_owner);
>> -            pg_owner = NULL;
>> -        }
>>          break;
>>      }
>>  
>> @@ -3008,6 +2997,13 @@ long do_mmuext_op(
>>          goto out;
>>      }
>>  
>> +    rc = xsm_mmuext_op(d, pg_owner);
>> +    if ( rc )
>> +    {
>> +        rcu_unlock_domain(pg_owner);
>> +        goto out;
>> +    }
>> +
> 
> While this part is fine, ...
> 
>>      for ( i = 0; i < count; i++ )
>>      {
>>          if ( hypercall_preempt_check() )
>> @@ -3483,11 +3479,6 @@ long do_mmu_update(
>>              rc = -EINVAL;
>>              goto out;
>>          }
>> -        if ( !IS_PRIV_FOR(d, pt_owner) )
>> -        {
>> -            rc = -ESRCH;
>> -            goto out;
>> -        }
> 
> ... this one isn't (at least in conjunction with them all becoming
> indirect calls unconditionally) - you replace a single validation per
> set of requests with one validation per request.
> 
> Jan
> 
> 

Is it still a problem if the check is inlined? If so, I could add an
additional XSM hook where the old IS_PRIV check was done, and make the
check inside the loop an inlined noop in the XSM-disabled case.

-- 
Daniel De Graaf
National Security Agency