Re: [Xen-devel] [PATCH RFC] xsm: use XSM instead of IS_PRIV where duplicated

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 07/31/2012 03:18 AM, Jan Beulich wrote:
>>>> On 30.07.12 at 21:49, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -4790,8 +4790,10 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) 
>> arg)
>>          XEN_GUEST_HANDLE(e820entry_t) buffer;
>>          unsigned int i;
>>  
>> +#ifndef XSM_ENABLE
>>          if ( !IS_PRIV(current->domain) )
>>              return -EINVAL;
>> +#endif
> 
> This recurring a number of times probably warrants some
> abstraction, to avoid the #ifdef-ery?
> 
>> --- a/xen/arch/x86/physdev.c
>> +++ b/xen/arch/x86/physdev.c
>> @@ -430,12 +430,15 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) 
>> arg)
>>          ret = -EFAULT;
>>          if ( copy_from_guest(&apic, arg, 1) != 0 )
>>              break;
>> -        ret = -EPERM;
>> -        if ( !IS_PRIV(v->domain) )
>> -            break;
>> +#ifdef XSM_ENABLE
>>          ret = xsm_apic(v->domain, cmd);
>>          if ( ret )
>>              break;
>> +#else
>> +        ret = -EPERM;
>> +        if ( !IS_PRIV(v->domain) )
>> +            break;
>> +#endif
> 
> Can't this be moved into the dummy stub, just like for other
> cases?
> 
> Jan
> 

I think the best solution here is to eliminate the explicit IS_PRIV checks
and have the XSM header switch between IS_PRIV and the XSM hooks depending
on XSM_ENABLE - all the calling code will look like the #ifdef XSM_ENABLE
version.

This ends up duplicating a lot of code between xsm/dummy.c and xsm/xsm.h,
so I'm considering doing something like this:

/* dummy.h: */

static int XSM_DEFAULT(apic)(struct domain *d, int cmd)
{
    if ( !IS_PRIV(current->domain) )
        return -EPERM;
    return 0;
}

/* dummy.c: */

#define XSM_DEFAULT(x) dummy_ ## x
#include "xsm/dummy.h"

struct xsm_operations dummy_xsm_ops;
/* set_to_dummy_if_null calls to populate */

/* xsm.h: */

#ifdef XSM_ENABLE
/* existing xsm_* inline wrappers around xsm_call */
#else
#define XSM_DEFAULT(x) inline xsm_ ## x
#include "xsm/dummy.h"
#endif

This would also allow the compiler to catch XSM function omissions in
dummy.c, which is currently not done - leaving them to be caught via
null function pointer dereferences at runtime when XSM is compiled but
no policy is loaded.

-- 
Daniel De Graaf
National Security Agency

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