Re: [Xen-devel] [PATCH 2/2] xen/gnt{dev, alloc}: reserve event channels for notify

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2011-10-18 at 22:04 +0100, Daniel De Graaf wrote:
> When using the unmap notify ioctl, the event channel used for
> notification needs to be reserved to avoid it being deallocated prior to
> sending the notification.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> ---
>  drivers/xen/gntalloc.c |   14 +++++++++++++-
>  drivers/xen/gntdev.c   |   11 +++++++++++
>  2 files changed, 24 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/xen/gntalloc.c b/drivers/xen/gntalloc.c
> index f6832f4..a739fb1 100644
> --- a/drivers/xen/gntalloc.c
> +++ b/drivers/xen/gntalloc.c
> @@ -178,8 +178,10 @@ static void __del_gref(struct gntalloc_gref *gref)
>               tmp[gref->notify.pgoff] = 0;
>               kunmap(gref->page);
>       }
> -     if (gref->notify.flags & UNMAP_NOTIFY_SEND_EVENT)
> +     if (gref->notify.flags & UNMAP_NOTIFY_SEND_EVENT) {
>               notify_remote_via_evtchn(gref->notify.event);
> +             evtchn_put(gref->notify.event);
> +     }
>  
>       gref->notify.flags = 0;
>  
> @@ -396,6 +398,16 @@ static long gntalloc_ioctl_unmap_notify(struct 
> gntalloc_file_private_data *priv,
>               goto unlock_out;
>       }
>  
> +     if (op.action & UNMAP_NOTIFY_SEND_EVENT) {
> +             if (evtchn_get(op.event_channel_port)) {
> +                     rc = -EINVAL;
> +                     goto unlock_out;
> +             }
> +     }
> +
> +     if (gref->notify.flags & UNMAP_NOTIFY_SEND_EVENT)
> +             evtchn_put(gref->notify.event);
> +

If the gref gets torn down here won't we notify and drop the reference
on the wrong evtchn, leading to a double free? If we defer the drop
until after gref->notify.event has been updated then this goes away.

Ian.

>       gref->notify.flags = op.action;
>       gref->notify.pgoff = pgoff;
>       gref->notify.event = op.event_channel_port;
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index f914b26..cfcc890 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -190,6 +190,7 @@ static void gntdev_put_map(struct grant_map *map)
>  
>       if (map->notify.flags & UNMAP_NOTIFY_SEND_EVENT) {
>               notify_remote_via_evtchn(map->notify.event);
> +             evtchn_put(map->notify.event);
>       }
>  
>       if (map->pages) {
> @@ -596,6 +597,16 @@ static long gntdev_ioctl_notify(struct gntdev_priv 
> *priv, void __user *u)
>               goto unlock_out;
>       }
>  
> +     if (op.action & UNMAP_NOTIFY_SEND_EVENT) {
> +             if (evtchn_get(op.event_channel_port)) {
> +                     rc = -EINVAL;
> +                     goto unlock_out;
> +             }
> +     }
> +
> +     if (map->notify.flags & UNMAP_NOTIFY_SEND_EVENT)
> +             evtchn_put(map->notify.event);
> +
>       map->notify.flags = op.action;
>       map->notify.addr = op.index - (map->index << PAGE_SHIFT);
>       map->notify.event = op.event_channel_port;



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel