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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.08.12 at 16:32, 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 @@ int do_mmuext_op(
>          goto out;
>      }
>  
> +    rc = xsm_mmuext_op(d, pg_owner);

Given the above, this could be

xsm_mmuext_op(dom0, DOMID_{IO,XEN});

yet ...

> +    if ( rc )
> +    {
> +        rcu_unlock_domain(pg_owner);
> +        goto out;
> +    }
> +
>      for ( i = 0; i < count; i++ )
>      {
>          if ( hypercall_preempt_check() )
> @@ -3483,11 +3479,6 @@ int do_mmu_update(
>              rc = -EINVAL;
>              goto out;
>          }
> -        if ( !IS_PRIV_FOR(d, pt_owner) )
> -        {
> -            rc = -ESRCH;
> -            goto out;
> -        }
>      }
>  
>      if ( (pg_owner = get_pg_owner((uint16_t)foreigndom)) == NULL )
> @@ -3643,7 +3634,7 @@ int do_mmu_update(
>              mfn = req.ptr >> PAGE_SHIFT;
>              gpfn = req.val;
>  
> -            rc = xsm_mmu_machphys_update(d, mfn);
> +            rc = xsm_mmu_machphys_update(d, pg_owner, mfn);
>              if ( rc )
>                  break;
>  
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -803,19 +803,35 @@ static XSM_DEFAULT(int, domain_memory_map) (struct 
> domain *d)
>  }
>  
>  static XSM_DEFAULT(int, mmu_normal_update) (struct domain *d, struct domain 
> *t,
> -                                    struct domain *f, intpte_t fpte)
> +                                            struct domain *f, intpte_t fpte)
>  {
> +    if ( d != t && !IS_PRIV_FOR(d, t) )
> +        return -EPERM;
> +    if ( d != f && !IS_PRIV_FOR(d, f) )
> +        return -EPERM;
>      return 0;
>  }
>  
> -static XSM_DEFAULT(int, mmu_machphys_update) (struct domain *d, unsigned 
> long mfn)
> +static XSM_DEFAULT(int, mmu_machphys_update) (struct domain *d, struct 
> domain *f,
> +                                              unsigned long mfn)
>  {
> +    if ( d != f && !IS_PRIV_FOR(d, f) )
> +        return -EPERM;
> +    return 0;
> +}
> +
> +static XSM_DEFAULT(int, mmuext_op) (struct domain *d, struct domain *f)
> +{
> +    if ( d != f && !IS_PRIV_FOR(d, f) )
> +        return -EPERM;

... Dom0 is neither privileged for DOM_IO nor for DOM_XEN.

>      return 0;
>  }
>  
>  static XSM_DEFAULT(int, update_va_mapping) (struct domain *d, struct domain 
> *f, 
>                                                              l1_pgentry_t pte)
>  {
> +    if ( d != f && !IS_PRIV_FOR(d, f) )
> +        return -EPERM;
>      return 0;
>  }
>  

Didn't check the other cases in any detail.

Jan


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