list_moved() appears to only work reliably if its argument is a non-empty
list. This is because list_moved dereferences the next and prev fields
of the list, which for an empty list are the address of the list itself,
and list_moved() is intended to be called when the address of the list has
changed.
This patch updates the only user of list_moved() that I was able to find
my temporarily setting the next field to NULL if it is empty and then
re-initialising the list rather than calling list_moved() after
reallocation has occurred.
An test update is also provided to exercise this change. Without
the code change the updated test cases a segmentation when the
buckets parameter of ofputil_put_ofp11_bucket() is dereferenced.
Signed-off-by: Simon Horman <simon.hor...@netronome.com>
---
* Although somewhat cure the approach of setting the next field to NULL
seems far less dangerous than trying to update the list infrastructure
to handle this case.
I also toyed with using a temporary bitmap to track empty buckets.
That approach worked but seemed to be more overhead and no cleaner
than the one taken by this patch.
---
lib/ofp-parse.c | 11 ++++++++++-
tests/ofproto.at | 13 +++++++++++--
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 9acf6a4..9a5df3b 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1415,9 +1415,18 @@ parse_ofp_group_mod_file(const char *file_name, uint16_t
command,
if (*n_gms >= allocated_gms) {
size_t i;
+ for (i = 0; i < *n_gms; i++) {
+ if (list_is_empty(&(*gms)[i].buckets)) {
+ (*gms)[i].buckets.next = NULL;
+ }
+ }
*gms = x2nrealloc(*gms, &allocated_gms, sizeof **gms);
for (i = 0; i < *n_gms; i++) {
- list_moved(&(*gms)[i].buckets);
+ if ((*gms)[i].buckets.next) {
+ list_moved(&(*gms)[i].buckets);
+ } else {
+ list_init(&(*gms)[i].buckets);
+ }
}
}
error = parse_ofp_group_mod_str(&(*gms)[*n_gms], command, ds_cstr(&s),
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 2a2111f..39978e4 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -286,16 +286,25 @@ dnl Actions definition listed in both supported formats
(w/ actions=)
AT_SETUP([ofproto - del group (OpenFlow 1.1)])
OVS_VSWITCHD_START
AT_DATA([groups.txt], [dnl
+group_id=1231,type=all
+group_id=1232,type=all
+group_id=1233,type=all
group_id=1234,type=all,bucket=output:10
group_id=1235,type=all,bucket=actions=output:10
])
AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-groups br0 groups.txt])
AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn dump-groups br0 ], [0], [stdout])
-AT_CHECK([STRIP_XIDS stdout], [0], [dnl
-OFPST_GROUP_DESC reply (OF1.1):
+AT_CHECK([STRIP_XIDS stdout | sort], [0], [dnl
+ group_id=1231,type=all
+ group_id=1232,type=all
+ group_id=1233,type=all
group_id=1234,type=all,bucket=actions=output:10
group_id=1235,type=all,bucket=actions=output:10
+OFPST_GROUP_DESC reply (OF1.1):
])
+AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn del-groups br0 group_id=1231])
+AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn del-groups br0 group_id=1232])
+AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn del-groups br0 group_id=1233])
AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn del-groups br0 group_id=1234])
AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn dump-groups br0], [0], [stdout])
AT_CHECK([STRIP_XIDS stdout], [0], [dnl
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
