Re: [Xen-devel] [PATCH 17/20] arch/x86: use XSM hooks for get_pg_owner access checks

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 09/11/2012 03:55 AM, Jan Beulich wrote:
>>>> On 10.09.12 at 21:49, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> 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

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