[ovs-dev] [PATCH] ovs-ofctl: Fix use-after-free error.

[Ben Pfaff]

Thanks, I pushed this.

On Mon, Jul 16, 2012 at 03:16:38PM -0500, Ethan Jackson wrote:
> Looks good to me, thanks.
> 
> Ethan
> 
> On Mon, Jul 16, 2012 at 12:24 PM, Ben Pfaff <blp at nicira.com> wrote:
> > Commit 4ce9c31573 (ovs-ofctl: Factor code out of read_flows_from_switch().)
> > introduced a use-after-free error, fixed by this change.
> >
> > Also adds a unit test for "ovs-ofctl diff-flows" that would have found the
> > problem.  (The bug report cited "diff-flows" but this bug was present in
> > dump-flows as well because they share common code.)
> >
> > Bug #12461.
> > Reported-by: James Schmidt <jschmidt at nicira.com>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  tests/ovs-ofctl.at    |   32 ++++++++++++++++++++++++++++++++
> >  utilities/ovs-ofctl.c |    1 +
> >  2 files changed, 33 insertions(+), 0 deletions(-)
> >
> > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> > index d456734..7233a66 100644
> > --- a/tests/ovs-ofctl.at
> > +++ b/tests/ovs-ofctl.at
> > @@ -1446,3 +1446,35 @@ AT_CHECK(
> >  ])
> >  OVS_VSWITCHD_STOP
> >  AT_CLEANUP
> > +
> > +AT_SETUP([ovs-ofctl diff-flows])
> > +OVS_VSWITCHD_START
> > +
> > +# Prints the integers from $1 to $2, increasing by $3 (default 1) on stdout.
> > +seq () {
> > +    while test $1 -le $2; do
> > +        echo $1
> > +        set `expr $1 + ${3-1}` $2 $3
> > +    done
> > +}
> > +
> > +# Add tons of flows to br0.
> > +for i in `seq 0 1023`; do echo "dl_vlan=$i,actions=drop"; done > add-flows.txt
> > +AT_CHECK([ovs-ofctl add-flows br0 add-flows.txt])
> > +
> > +# Dump them and compare against what we expect by hand, then with diff-flows.
> > +for i in `seq 0 1023`; do echo " dl_vlan=$i actions=drop"; done | sort > expout
> > +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sed '/NXST_FLOW/d' | sort],
> > +  [0], [expout])
> > +AT_CHECK([ovs-ofctl diff-flows br0 add-flows.txt])
> > +
> > +# Remove even-numbered flows, compare again.
> > +for i in `seq 0 1023 2`; do echo "dl_vlan=$i"; done > del-flows.txt
> > +AT_CHECK([ovs-ofctl del-flows br0 - < del-flows.txt])
> > +for i in `seq 0 1023 2`; do echo "+dl_vlan=$i actions=drop"; done | sort > expout
> > +AT_CHECK([ovs-ofctl diff-flows br0 add-flows.txt | sort], [0], [expout])
> > +for i in `seq 0 1023 2`; do echo "-dl_vlan=$i actions=drop"; done | sort > expout
> > +AT_CHECK([ovs-ofctl diff-flows add-flows.txt br0 | sort], [0], [expout])
> > +
> > +OVS_VSWITCHD_STOP
> > +AT_CLEANUP
> > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > index f925d84..4ad1acb 100644
> > --- a/utilities/ovs-ofctl.c
> > +++ b/utilities/ovs-ofctl.c
> > @@ -1903,6 +1903,7 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32 send_xid,
> >          case EOF:
> >              flags = ((const struct ofp_stats_msg *) reply->l2)->flags;
> >              ofpbuf_delete(reply);
> > +            reply = NULL;
> >              if (!(flags & htons(OFPSF_REPLY_MORE))) {
> >                  *replyp = NULL;
> >                  return false;
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev