[ovs-dev] [PATCH] bridge: Avoid use-after-free with VLAN splinters and multiple bridges.

[Ben Pfaff]

Thanks, I pushed this to master and branch-1.4.

On Fri, Dec 09, 2011 at 05:22:37PM -0800, Justin Pettit wrote:
> Looks good.
> 
> --Justin
> 
> 
> On Dec 9, 2011, at 3:59 PM, Ben Pfaff wrote:
> 
> > The VLAN splinters feature uses a "pool" to track and free allocated
> > blocks.  There's only one pool, but the implementation was freeing all of
> > the blocks in it for every bridge during reconfiguration, not just once for
> > each reconfiguration, so caused a use-after-free when there was more than
> > one bridge and a bridge other than the last one in the list of bridges had
> > a VLAN splinter port.
> > 
> > Bug #8671.
> > Reported-by: Michael Mao <mmao at nicira.com>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > vswitchd/bridge.c |    6 ++++--
> > 1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> > index f79b69c..adf01f7 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -3405,6 +3405,10 @@ collect_splinter_vlans(const struct ovsrec_open_vswitch *ovs_cfg)
> >     struct bridge *br;
> >     size_t i;
> > 
> > +    /* Free space allocated for synthesized ports and interfaces, since we're
> > +     * in the process of reconstructing all of them. */
> > +    free_registered_blocks();
> > +
> >     splinter_vlans = NULL;
> >     sset_init(&splinter_ifaces);
> >     for (i = 0; i < ovs_cfg->n_bridges; i++) {
> > @@ -3572,8 +3576,6 @@ add_vlan_splinter_ports(struct bridge *br,
> > {
> >     size_t i;
> > 
> > -    free_registered_blocks();
> > -
> >     /* We iterate through 'br->cfg->ports' instead of 'ports' here because
> >      * we're modifying 'ports'. */
> >     for (i = 0; i < br->cfg->n_ports; i++) {
> > -- 
> > 1.7.4.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>