WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

On Tue, Oct 18, 2011 at 05:04:11PM -0400, 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);

So.. I could have some fun by doing this in the userspace:
  for (j = 0; j< 2;j++) {
      for (i = 0; i < 65534; i++) {
         struct ioctl_gntalloc_unmap_notify uarg = {
                 .index = arg.index + offsetof(struct shr_page, notifies[0]),
                 .action =UNMAP_NOTIFY_SEND_EVENT,
                 .event_channel_port = i,
         };
         rv = ioctl(a_fd, IOCTL_GNTALLOC_SET_UNMAP_NOTIFY, &uarg);
        }
  }

And cause the event channel refcnt to be set to zero and free it. And then
causing the box to die - as the event channels for the physical IRQ might have
gotten free-ed.


Hm.. Perhaps the gntalloc and gntdev should keep track of which event channels
are OK to refcnt? Something like a whitelist? Granted at that point the 
refcounting
could as well be done by the API that sets up the event channels from the 
userspace.

So the evtchn_ioctl is pretty smart. It uses "get_port_user" to get the list
of events that belong to this user (and have been handed out). I think you
need to use that in the gntalloc to double-check that the event channel is not
one of the kernel type.

> +     }
>  
>       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);
> +
>       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)

So notify.flags has not been set yet? That looks to be done later?

Or is this in case of the user doing

 uargs.action = UNMAP_NOTIFY_SEND_EVENT;
 ioctl(.., IOCTL_GNTALLOC_SET_UNMAP_NOTIFY, &uarg);
 uargs.action = UNAMP_NOTIFY_CLEAR_BYTE;
 ioctl(.., IOCTL_GNTALLOC_SET_UNMAP_NOTIFY, &uarg);

 and we want to preserve the "old" flags before swapping over to the
new?

> +             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;
> -- 
> 1.7.6.4

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

<Prev in Thread] Current Thread [Next in Thread>