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

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

On Wed, 2011-10-19 at 15:23 +0100, Daniel De Graaf wrote:
> On 10/19/2011 05:24 AM, Ian Campbell wrote:
> > 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.
> 
> This evtchn_put will only be called in the case where the unmap_notify
> is being changed and already had an event channel reference. This reference
> must be dropped prior to changing gref->notify.event or we will leak
> the old event channel.

More importantly I see now that there is a lock protecting all this
stuff so everything is ok.

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

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