Re: [PATCH] common: don't require use of DOMID_SELF

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 14/01/2021 14:02, Jan Beulich wrote:
> It's not overly difficult for a domain to figure out its ID, so
> requiring the use of DOMID_SELF in a very limited set of places isn't
> really helpful towards keeping the ID opaque to the guest.
So I agree that a domid can be figured out really easily today and in 
principle it would be fine to relax it.
However, most of the guest OSes will care about running on older Xen 
versions. Therefore they are not going to be able to use this relaxation.
So I am not entirely convinced the relaxation is actually worth it for 
existing hypercalls.
Anyway, if we decide to relax it, then I think we should update the 
public headers because an OS using this relaxation will not work on 
older Xen. A developper will not be able to know that without looking at 
the implementation.
Cheers,

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -2776,15 +2776,19 @@ struct gnttab_copy_buf {
>   static int gnttab_copy_lock_domain(domid_t domid, bool is_gref,
>                                      struct gnttab_copy_buf *buf)
>   {
> -    /* Only DOMID_SELF may reference via frame. */
> -    if ( domid != DOMID_SELF && !is_gref )
> -        return GNTST_permission_denied;
> -
>       buf->domain = rcu_lock_domain_by_any_id(domid);
>   
>       if ( !buf->domain )
>           return GNTST_bad_domain;
>   
> +    /* Only the local domain may reference via frame. */
> +    if ( buf->domain != current->domain && !is_gref )
> +    {
> +        rcu_unlock_domain(buf->domain);
> +        buf->domain = NULL;
> +        return GNTST_permission_denied;
> +    }
> +
>       buf->ptr.domid = domid;
>   
>       return GNTST_okay;
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -2566,13 +2566,7 @@ __initcall(register_heap_trigger);
>   
>   struct domain *get_pg_owner(domid_t domid)
>   {
> -    struct domain *pg_owner = NULL, *curr = current->domain;
> -
> -    if ( unlikely(domid == curr->domain_id) )
> -    {
> -        gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
> -        goto out;
> -    }
> +    struct domain *pg_owner;
>   
>       switch ( domid )
>       {
> @@ -2590,7 +2584,6 @@ struct domain *get_pg_owner(domid_t domi
>           break;
>       }
>   
> - out:
>       return pg_owner;
>   }
>   
--
Julien Grall