On Thu, Sep 05, 2013 at 02:19:13PM +0900, Simon Horman wrote:
> struct ofputil_group_stats has an arbitrary limit
> of 16 buckets for which it can record statistics.
> However the code does not appear to enforce this
> limit and it seems to me that the code could overflow.
>
> This patch aims to remove the arbitrary limit by
> changing the 'bucket_stats' field of struct ofputil_group_stats
> from a fixed length array to a pointer whose storage is allocated and freed
> as necessary.
>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
>
> ---
> v3
> * As suggested by Ben Pfaff
> - Vastly simplify the change by using an explicit pointer for
> the 'bucket_stats' member of struct ofputil_group_stats rather
> than implicit variable length array appended to the end of the
> structure.
Thanks!
It looked to me that ofp_print_group_stats() shouldn't free
gs.bucket_stats on error because in that case gs.bucket_stats won't be
properly initialized, so I changed that. And then I changed
ofputil_decode_group_stats_reply() to always initialize
gs.bucket_stats as a second measure of protection.
I folded in the following and will apply patches 1 and 2 soon.
I'll review patches 3 through 5 tomorrow during the hackathon.
(Thanks so much for participating!)
--8<--------------------------cut here-------------------------->8--
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index a794e70..6433dd0 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2229,7 +2229,6 @@ ofp_print_group_stats(struct ds *s, const struct
ofp_header *oh)
if (retval != EOF) {
ds_put_cstr(s, " ***parse error***");
}
- free(gs.bucket_stats);
break;
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6573025..8ac9186 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -5480,7 +5480,8 @@ ofputil_decode_group_stats_request(const struct
ofp_header *request,
}
/* Converts a group stats reply in 'msg' into an abstract ofputil_group_stats
- * in 'gs'.
+ * in 'gs'. Assigns freshly allocated memory to gs->bucket_stats for the
+ * caller to eventually free.
*
* Multiple group stats replies can be packed into a single OpenFlow message.
* Calling this function multiple times for a single 'msg' iterates through the
@@ -5501,6 +5502,7 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
size_t length;
size_t i;
+ gs->bucket_stats = NULL;
error = (msg->l2
? ofpraw_decode(&raw, msg->l2)
: ofpraw_pull(&raw, msg));
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
