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 <horms@verge.net.au>
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 92fb40f..2ada88d 100644 (file)
--- 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 f2ddda9..441cdf2 100644 (file)
--- 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