Re: [PATCH v4 03/34] xsm/xen_version: Add XSM for the xen_version hypercall

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

On 03/22/2016 12:10 PM, Konrad Rzeszutek Wilk wrote:
> On Mon, Mar 21, 2016 at 05:22:09AM -0600, Jan Beulich wrote:
>>>>> On 18.03.16 at 18:26, <konrad.wilk@oracle.com> wrote:
>>> On Fri, Mar 18, 2016 at 05:55:55AM -0600, Jan Beulich wrote:
>>>>>>> On 15.03.16 at 18:56, <konrad.wilk@oracle.com> wrote:
>>>>> @@ -223,12 +224,15 @@ void __init do_initcalls(void)
>>>>>   /*
>>>>>    * Simple hypercalls.
>>>>>    */
>>>>> -
>>>>>   DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>>
>>>> Please retain the blank line, as it relates to more than just this
>>>> one function.
>>>
>>> Done! (stray change).
>>
>> Considering this I'm not puzzled by ...
>>
>>>       case XENVER_guest_handle:
>>> -        if ( copy_to_guest(arg, current->domain->handle,
>>> -                           ARRAY_SIZE(current->domain->handle)) )
>>> +    {
>>> +        xen_domain_handle_t hdl;
>>> +
>>> +        if ( deny )
>>> +            memset(&hdl, 0, ARRAY_SIZE(hdl));
>>> +
>>> +        BUILD_BUG_ON(ARRAY_SIZE(current->domain->handle) != ARRAY_SIZE(hdl));
>>> +
>>> +        if ( copy_to_guest(arg, deny ? hdl : current->domain->handle,
>>> +                           ARRAY_SIZE(hdl) ) )
>>>               return -EFAULT;
>>>           return 0;
>>> -
>>> +    }
>>>       case XENVER_commandline:
>>
>> ... this.
>
> Wow. That is some sharp eyes!
>>
>>> --- a/xen/include/xsm/dummy.h
>>> +++ b/xen/include/xsm/dummy.h
>>> @@ -727,3 +727,27 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int
>>>   }
>>>
>>>   #endif /* CONFIG_X86 */
>>> +
>>> +#include <public/version.h>
>>> +static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op)
>>> +{
>>> +    XSM_ASSERT_ACTION(XSM_OTHER);
>>> +    switch ( op )
>>> +    {
>>> +    case XENVER_version:
>>> +    case XENVER_platform_parameters:
>>> +    case XENVER_get_features:
>>> +        /* The sub-ops ignores the permission check and returns data. */
>>
>> ignore ... and return ...
>>
>> With those minor things addressed I think the patch can have my ack.
>
> Thank you!
>
> Now I just need Daniel's Ack again.
>
>>From 1ccf59abdd2cd9228f0159dce77fe404d98c7300 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Fri, 11 Mar 2016 21:40:43 -0500
> Subject: [PATCH] xsm/xen_version: Add XSM for most of xen_version hypercall
>
> Most of XENVER_* have now an XSM check for their sub-ops.
>
> The subop for XENVER_commandline is now a priviliged operation.
> To not break guests we still return an string - but it is
> just '<denied>\0'.
>
> The XENVER_[version|platform_parameters|get_features] - will
> always return an value to the guest.
>
> The rest: XENVER_[extraversion|capabilities|page_size|
> guest_handle|changeset| compile_info] behave as before -
> allowed by default for all guests if using the XSM default
> policy or with the dummy one. And if the system admin
> wants to curtail access to some of them - they can do
> that now with a non-default XSM policy.
>
> Also we add a local variable block.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Replied to the wrong email before; this one is actually:

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

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