Re: [PATCH 18/23] arch/x86: Add missing mem_sharing XSM hooks

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

On 09/27/2012 09:09 AM, Tim Deegan wrote:
> Cc'ing Joe, the author of the original check I'm talking about below. 
> 
> At 11:23 -0400 on 17 Sep (1347881020), Daniel De Graaf wrote:
>> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
>> index 24e2d93..7062f02 100644
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -1447,10 +1447,8 @@ long arch_do_domctl(
>>          d = rcu_lock_domain_by_id(domctl->domain);
>>          if ( d != NULL )
>>          {
>> -            ret = xsm_mem_event(d);
>> -            if ( !ret )
>> -                ret = mem_event_domctl(d, &domctl->u.mem_event_op,
>> -                                       guest_handle_cast(u_domctl, void));
>> +            ret = mem_event_domctl(d, &domctl->u.mem_event_op,
>> +                                   guest_handle_cast(u_domctl, void));
>>              rcu_unlock_domain(d);
>>              copy_to_guest(u_domctl, domctl, 1);
>>          } 
>> @@ -1506,7 +1504,7 @@ long arch_do_domctl(
>>          d = rcu_lock_domain_by_id(domctl->domain);
>>          if ( d != NULL )
>>          {
>> -            ret = xsm_mem_event(d);
>> +            ret = xsm_mem_event_setup(d);
>>              if ( !ret ) {
>>                  p2m = p2m_get_hostp2m(d);
>>                  p2m->access_required = domctl->u.access_required.access_required;
> 
> [...]
> 
>> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
>> index 626a332..5fb0afe 100644
>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -551,16 +551,37 @@ static XSM_DEFAULT(int, hvm_inject_msi) (struct domain *d)
>>      return 0;
>>  }
>>  
>> -static XSM_DEFAULT(int, mem_event) (struct domain *d)
>> +static XSM_DEFAULT(int, mem_event_setup) (struct domain *d)
>>  {
>>      return 0;
>>  }
> 
> I think this ought to be at least IS_PRIV_FOR.  I can see the original
> code allowed all callers to use it, but surely it ought to be only for
> the tools.  Since only the tools can actually set the mem-access rights
> (and so this is pretty much a noop) I don't think this causes any
> substantial problem but we might as well adjust it anyway.
> 
> Tim.

Because this is a domctl, it already requires IS_PRIV as checked by 
xsm_domctl (and was already checked before this series).

-- 
Daniel De Graaf
National Security Agency