[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 11/18] xen: use XSM instead of IS_PRIV where duplicated



>>> On 06.08.12 at 16:32, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -54,6 +54,26 @@ long arch_do_domctl(
>  
>      switch ( domctl->cmd )
>      {
> +    /* TODO: the following do not have XSM hooks yet */
> +    case XEN_DOMCTL_set_cpuid:
> +    case XEN_DOMCTL_suppress_spurious_page_faults:
> +    case XEN_DOMCTL_debug_op:
> +    case XEN_DOMCTL_gettscinfo:
> +    case XEN_DOMCTL_settscinfo:
> +    case XEN_DOMCTL_audit_p2m:
> +    case XEN_DOMCTL_gdbsx_guestmemio:
> +    case XEN_DOMCTL_gdbsx_pausevcpu:
> +    case XEN_DOMCTL_gdbsx_unpausevcpu:
> +    case XEN_DOMCTL_gdbsx_domstatus:
> +    /* getpageframeinfo[23] will leak XEN_DOMCTL_PFINFO_XTAB on target GFNs 
> */

Is that to state that the patch introduces a leak here? Or are you
trying to carefully tell us you spotted a problem in the existing
code?

> +    case XEN_DOMCTL_getpageframeinfo2:
> +    case XEN_DOMCTL_getpageframeinfo3:
> +        if ( !IS_PRIV(current->domain) )
> +            return -EPERM;
> +    }
> +
> +    switch ( domctl->cmd )
> +    {
>  
>      case XEN_DOMCTL_shadow_op:
>      {
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3366,12 +3366,12 @@ static int hvmop_set_pci_intx_level(
>      if ( (op.domain > 0) || (op.bus > 0) || (op.device > 31) || (op.intx > 
> 3) )
>          return -EINVAL;
>  
> -    rc = rcu_lock_remote_target_domain_by_id(op.domid, &d);
> -    if ( rc != 0 )
> -        return rc;
> +    d = rcu_lock_domain_by_id(op.domid);
> +    if ( d == NULL )
> +        return -ESRCH;
>  
>      rc = -EINVAL;
> -    if ( !is_hvm_domain(d) )
> +    if ( d == current->domain || !is_hvm_domain(d) )

What's wrong with rcu_lock_remote_target_domain_by_id() here
and in other places below? I think this minimally would deserve
a comment in the patch description, the more that this huge a
patch is already bad enough to look at.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.