Re: [Xen-devel] [PATCH][NET] front: cleanup some error paths

[Chris Wright <chrisw@xxxxxxxxxxxx>]

* Keir Fraser (Keir.Fraser@xxxxxxxxxxxx) wrote:
> 
> On 8 Jun 2006, at 20:22, Chris Wright wrote:
> 
> >There's a small leak on a couple error paths in setup_device().
> >While there rearrange the ring setup order slightly to simplify error
> >path since netif_free() will cleanup once ring_ref is valid.  And use
> >get_zeroed_page() instead of __get_free_page()/memset().  Handle error 
> >if
> >bind_evtchn_to_irqhandler() fails, as bad info->irq value is likely to
> >cause oops later.  In create_device(), gnttab_free_grant_references()
> >is accidentally called twice on tx_head during cleanup from failed
> >gnttab_alloc_grant_references() on rx_head, which could corrupt
> >gnttab_free_count.
> 
> Applied, but I don't think it's a complete fix. For example, shouldn't 
> txs/rxs be freed if bind_evtchn_to_irqhandler() fails? There should 
> probably be more test-and-free cases on the fail path.

They are.  netif_free() handles it.  Just not until the ring_ref is
valid (in netif_free()->netif_disconnect_backend()->end_acess()).

Alternative is smth like this (psuedo-patch):

end_access(int ref, void *page)
{
        if (ref != GRANT_INVALID_REF)
                gnttab_end_foreign_access(ref, 0, (unsigned long)page);
+       else
+               if (page)
+                       free_page((unsigned long)page);
}

But that seems a bit obtuse to me and possibly error prone, which is
why I chose the method in the patch.

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