From 65bfce4ae0603c8c3576acc5b3cd1ba5afff4c00 Mon Sep 17 00:00:00 2001 From: Simon Horman Date: Mon, 7 Apr 2014 16:43:04 +0000 Subject: [PATCH] ofp-parse: Handle buffer resize when parsing actions A call to ofpbuf_put() may cause the data of the passed to be resized and its base address to change. Thus the address returned by ofpbuf_put() should be used as the base address rather than relying on the base address prior to ofpbuf_put(). This avoids the following assertion in ofpact_update_len() from failing when ofpbuf_put() causes a resize in parse_note(), parse_noargs_dec_ttl(), parse_dec_ttl(). ovs_assert(ofpact == ofpacts->frame); This restores pointer updates removed by and resolves a regression introduced by cf3b7538666cd1ef ("ofpbuf: Abstract 'l2' pointer and document usage conventions."). Test cases have also been added to exercise this buffer resize in parse_note(), parse_noargs_dec_ttl(), parse_dec_ttl(). Signed-off-by: Simon Horman Signed-off-by: Jarno Rajahalme --- lib/ofp-parse.c | 3 +++ tests/ofproto-dpif.at | 50 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 92fb40f95..2ada88d06 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -291,6 +291,7 @@ parse_note(const char *arg, struct ofpbuf *ofpacts) } ofpbuf_put(ofpacts, &byte, 1); + note = ofpacts->frame; note->length++; arg += 2; @@ -399,6 +400,7 @@ parse_noargs_dec_ttl(struct ofpbuf *b) ids = ofpact_put_DEC_TTL(b); ofpbuf_put(b, &id, sizeof id); + ids = b->frame; ids->n_controllers++; ofpact_update_len(b, &ids->ofpact); } @@ -424,6 +426,7 @@ parse_dec_ttl(struct ofpbuf *b, char *arg) uint16_t id = atoi(cntr); ofpbuf_put(b, &id, sizeof id); + ids = b->frame; ids->n_controllers++; } if (!ids->n_controllers) { diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index f2ddda9d6..441cdf2db 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -336,6 +336,56 @@ ip,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00 OVS_VSWITCHD_STOP AT_CLEANUP +dnl A dec_ttl action at offset 32 in ofpacts will cause the ofpacts +dnl buffer to be resized just before pushing the id of the dec_ttl action. +dnl Thus the implementation must account for this by using the +dnl reallocated buffer rather than the original buffer. +dnl +dnl A number of similar rules are added to try and exercise +dnl xrealloc sufficiently that it returns a different base pointer +AT_SETUP([ofproto-dpif - dec_ttl without arguments at offset 32 in ofpacts]) +OVS_VSWITCHD_START +ADD_OF_PORTS([br0], [1]) +(for i in `seq 0 255`; do + printf "dl_src=10:11:11:11:11:%02x actions=output:1,output:1,output:1,dec_ttl,controller\n" $i + done) > flows.txt +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +dnl A dec_ttl action at offset 32 in ofpacts will cause the ofpacts +dnl buffer to be resized just before pushing the id of the dec_ttl action. +dnl Thus the implementation must account for this by using the +dnl reallocated buffer rather than the original buffer. +dnl +dnl A number of similar rules are added to try and exercise +dnl xrealloc sufficiently that it returns a different base pointer +AT_SETUP([ofproto-dpif - dec_ttl with arguments at offset 32 in ofpacts]) +OVS_VSWITCHD_START +ADD_OF_PORTS([br0], [1]) +(for i in `seq 0 255`; do + printf "dl_src=10:11:11:11:11:%02x actions=output:1,output:1,output:1,dec_ttl(1),controller\n" $i + done) > flows.txt +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +dnl A note action at offset 24 in ofpacts will cause the ofpacts +dnl buffer to be resized just before pushing the id of the dec_ttl action. +dnl Thus the implementation must account for this by using the +dnl reallocated buffer rather than the original buffer. +dnl +dnl A number of similar rules are added to try and exercise +dnl xrealloc sufficiently that it returns a different base pointer +AT_SETUP([ofproto-dpif - note at offset 24 in ofpacts]) +OVS_VSWITCHD_START +ADD_OF_PORTS([br0], [1]) +(for i in `seq 0 255`; do + printf "dl_src=10:11:11:11:11:%02x actions=output:1,output:1,note:ff,controller\n" $i + done) > flows.txt +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +OVS_VSWITCHD_STOP +AT_CLEANUP AT_SETUP([ofproto-dpif - output, OFPP_NONE ingress port]) OVS_VSWITCHD_START -- 2.43.0