[ovs-dev] [PATCH] ofproto: Fix bad memory access sending large numbers of port stats replies.

Ben Pfaff blp at nicira.com
Mon Apr 26 23:29:59 UTC 2010


Thanks.  I pushed this out.

On Mon, Apr 26, 2010 at 03:45:59PM -0700, Justin Pettit wrote:
> Looks good.
> 
> --Justin
> 
> 
> On Apr 26, 2010, at 3:40 PM, Ben Pfaff wrote:
> 
> > The append_stats_reply() function can modify its pointer argument, but
> > append_port_stat() was failing to propagate this change back to its own
> > caller.  So when append_stats_reply() did in fact modify it (which happens
> > when the 64 kB maximum OpenFlow message length was exceeded), the
> > handle_port_stats_request() function would then access freed memory on the
> > next call to append_port_stat().
> > 
> > Bug #2714.
> > Reported-by: Ram Jothikumar <rjothikumar at nicira.com>
> > Debugging help by Justin Pettit <jpettit at nicira.com>
> > ---
> > ofproto/ofproto.c |    8 ++++----
> > 1 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index 3f9cc90..6c93aea 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -2760,7 +2760,7 @@ handle_table_stats_request(struct ofproto *p, struct ofconn *ofconn,
> > 
> > static void
> > append_port_stat(struct ofport *port, uint16_t port_no, struct ofconn *ofconn, 
> > -                 struct ofpbuf *msg)
> > +                 struct ofpbuf **msgp)
> > {
> >     struct netdev_stats stats;
> >     struct ofp_port_stats *ops;
> > @@ -2770,7 +2770,7 @@ append_port_stat(struct ofport *port, uint16_t port_no, struct ofconn *ofconn,
> >      * netdev_get_stats() will log errors. */
> >     netdev_get_stats(port->netdev, &stats);
> > 
> > -    ops = append_stats_reply(sizeof *ops, ofconn, &msg);
> > +    ops = append_stats_reply(sizeof *ops, ofconn, msgp);
> >     ops->port_no = htons(odp_port_to_ofp_port(port_no));
> >     memset(ops->pad, 0, sizeof ops->pad);
> >     ops->rx_packets = htonll(stats.rx_packets);
> > @@ -2808,11 +2808,11 @@ handle_port_stats_request(struct ofproto *p, struct ofconn *ofconn,
> >         port = port_array_get(&p->ports, 
> >                 ofp_port_to_odp_port(ntohs(psr->port_no)));
> >         if (port) {
> > -            append_port_stat(port, ntohs(psr->port_no), ofconn, msg);
> > +            append_port_stat(port, ntohs(psr->port_no), ofconn, &msg);
> >         }
> >     } else {
> >         PORT_ARRAY_FOR_EACH (port, &p->ports, port_no) {
> > -            append_port_stat(port, port_no, ofconn, msg);
> > +            append_port_stat(port, port_no, ofconn, &msg);
> >         }
> >     }
> > 
> > -- 
> > 1.6.6.1
> > 
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
> 




More information about the dev mailing list