[ovs-dev] [PATCH] ofproto-dpif: Destroy facets on ofproto removal.

Ben Pfaff blp at nicira.com
Wed Sep 4 17:27:32 UTC 2013


On Wed, Sep 04, 2013 at 09:27:31AM -0700, Ethan Jackson wrote:
> Facets maintain a pointer to their owning ofproto-dpif, and therefore
> when an ofproto-dpif is destroyed all of their child facets should be
> destroyed along with it.  If they aren't, it's possible a subfacet
> looked up in the dpif-backer could access it's parent facet and
> therefore a defunct parent ofproto causing a segmentation fault.
> 
> Bug #19421.
> Bug #19423.
> Diagnosed-by: Ben Pfaff <blp at nicira.com>
> Signed-off-by: Ethan Jackson <ethan at nicira.com>

Thanks.

> +    ovs_rwlock_rdlock(&ofproto->facets.rwlock);
> +    cls_cursor_init(&cursor, &ofproto->facets, NULL);
> +    CLS_CURSOR_FOR_EACH_SAFE (facet, next_facet, cr, &cursor) {
> +        facet_remove(facet);
> +    }
> +    ovs_rwlock_unlock(&ofproto->facets.rwlock);

I'm pretty sure that the above will self-deadlock on
ofproto->facets.rwlock, because facet_remove() write-locks it.

My understanding is that the locking on this classifier is superfluous
anyway since only a single thread accesses it--it's only there to
suppress Clang lock warnings--so I just moved the unlock just past the
cls_cursor_init().

I'm going to test this a little, apply it, and backport it.



More information about the dev mailing list