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. v2 * No change --- lib/ofp-print.c | 4 ++++ lib/ofp-util.c | 1 + lib/ofp-util.h | 2 +- ofproto/ofproto.c | 4 ++++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 702d9c1..a794e70 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2223,11 +2223,13 @@ ofp_print_group_stats(struct ds *s, const struct ofp_header *oh) struct ofputil_group_stats gs; int retval; + gs.bucket_stats = NULL; retval = ofputil_decode_group_stats_reply(&b, &gs); if (retval) { if (retval != EOF) { ds_put_cstr(s, " ***parse error***"); } + free(gs.bucket_stats); break; } @@ -2252,6 +2254,8 @@ ofp_print_group_stats(struct ds *s, const struct ofp_header *oh) ds_put_format(s, "byte_count=%"PRIu64"", gs.bucket_stats[bucket_i].byte_count); } } + + free(gs.bucket_stats); } } diff --git a/lib/ofp-util.c b/lib/ofp-util.c index b181a0d..6573025 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -5557,6 +5557,7 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg, return OFPERR_OFPBRC_BAD_LEN; } + gs->bucket_stats = xmalloc(gs->n_buckets * sizeof *gs->bucket_stats); for (i = 0; i < gs->n_buckets; i++) { gs->bucket_stats[i].packet_count = ntohll(obc[i].packet_count); gs->bucket_stats[i].byte_count = ntohll(obc[i].byte_count); diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 6de2a05..eae8ed5 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -913,7 +913,7 @@ struct ofputil_group_stats { uint32_t duration_sec; /* UINT32_MAX if unknown. */ uint32_t duration_nsec; uint32_t n_buckets; - struct bucket_counter bucket_stats[16]; + struct bucket_counter *bucket_stats; }; /* Group features reply, independent of protocol. */ diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index dacbb71..b94fd8e 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -4786,6 +4786,8 @@ append_group_stats(struct ofgroup *group, struct list *replies) long long int now = time_msec(); int error; + ogs.bucket_stats = xmalloc(group->n_buckets * sizeof *ogs.bucket_stats); + error = (ofproto->ofproto_class->group_get_stats ? ofproto->ofproto_class->group_get_stats(group, &ogs) : EOPNOTSUPP); @@ -4802,6 +4804,8 @@ append_group_stats(struct ofgroup *group, struct list *replies) calc_duration(group->created, now, &ogs.duration_sec, &ogs.duration_nsec); ofputil_append_group_stats(replies, &ogs); + + free(ogs.bucket_stats); } static enum ofperr -- 1.8.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev