Re: [PATCH 5/8] xen/evtchn: don't close the static event channel.

[Julien Grall <julien@xxxxxxx>]

Hi,

On 22/06/2022 15:38, Rahul Singh wrote:
> Guest can request the Xen to close the event channels. Ignore the
> request from guest to close the static channels as static event channels
> should not be closed.
Why do you want to prevent the guest to close static ports? The problem 
I can see is...
[...]

> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 84f0055a5a..cedc98ccaf 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -294,7 +294,8 @@ void evtchn_free(struct domain *d, struct evtchn *chn)
>    * If port is zero get the next free port and allocate. If port is non-zero
>    * allocate the specified port.
>    */
> -int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc, evtchn_port_t port)
> +int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc, evtchn_port_t port,
> +                         bool is_static)
>   {
>       struct evtchn *chn;
>       struct domain *d;
> @@ -330,6 +331,7 @@ int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc, 
> evtchn_port_t port)
>       evtchn_write_lock(chn);
>   
>       chn->state = ECS_UNBOUND;
> +    chn->is_static = is_static;
>       if ( (chn->u.unbound.remote_domid = alloc->remote_dom) == DOMID_SELF )
>           chn->u.unbound.remote_domid = current->domain->domain_id;
>       evtchn_port_init(d, chn);
> @@ -368,7 +370,7 @@ static void double_evtchn_unlock(struct evtchn *lchn, 
> struct evtchn *rchn)
>    * allocate the specified lport.
>    */
>   int evtchn_bind_interdomain(evtchn_bind_interdomain_t *bind, struct domain 
> *ld,
> -                            evtchn_port_t lport)
> +                            evtchn_port_t lport, bool is_static)
>   {
>       struct evtchn *lchn, *rchn;
>       struct domain *rd;
> @@ -423,6 +425,7 @@ int evtchn_bind_interdomain(evtchn_bind_interdomain_t 
> *bind, struct domain *ld,
>       lchn->u.interdomain.remote_dom  = rd;
>       lchn->u.interdomain.remote_port = rport;
>       lchn->state                     = ECS_INTERDOMAIN;
> +    lchn->is_static                 = is_static;
>       evtchn_port_init(ld, lchn);
>       
>       rchn->u.interdomain.remote_dom  = ld;
> @@ -659,6 +662,9 @@ int evtchn_close(struct domain *d1, int port1, bool guest)
>           rc = -EINVAL;
>           goto out;
>       }
> +    /* Guest cannot close a static event channel. */
> +    if ( chn1->is_static && guest )
> +        goto out;
... at least the interdomain structure store pointer to the domain. I am 
a bit concerned that we would end up to leave dangling pointers (such as 
chn->u.interdomain.remote_domain) as evtchn_close() is also used while 
destroying the domain.
Also, AFAICT Xen will return 0 (i.e. success) to the caller. I think 
this is a mistake because we didn't close the port as requested.
Cheers,

--
Julien Grall