ovs-ofctl: Avoid read overrun in ofperr_decode_msg().

vconn_add_bundle_error() was keeping at most 64 bytes of an OpenFlow
error message, then it was passing it to ofperr_decode_msg(), which assumed
that the full message was available.  This led to a buffer overread.
There's no good reason why it was only keeping the first 64 bytes, so this
commit changes it to keep the whole error message, sidestepping the
problem.

struct vconn_bundle_error only existed for this special case, so remove it
in favor of a chain of ofpbufs.

Found via gcc's address sanitizer.

Reported-by: Lance Richardson <lrichard@redhat.com>
Fixes: 506c1ddb3404 ("vconn: Better bundle error management.")
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahlame <jarno@ovn.org>

1 files changed, 6 insertions, 4 deletions

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index dca9be3a5..95989eb11 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -699,16 +699,18 @@ static void
 bundle_print_errors(struct ovs_list *errors, struct ovs_list *requests,
                     const char *vconn_name)
 {
-    struct vconn_bundle_error *error, *next;
+    struct ofpbuf *error, *next;
     struct ofpbuf *bmsg;
 
     INIT_CONTAINER(bmsg, requests, list_node);
 
     LIST_FOR_EACH_SAFE (error, next, list_node, errors) {
+        const struct ofp_header *error_oh = error->data;
+        ovs_be32 error_xid = error_oh->xid;
         enum ofperr ofperr;
         struct ofpbuf payload;
 
-        ofperr = ofperr_decode_msg(&error->ofp_msg, &payload);
+        ofperr = ofperr_decode_msg(error_oh, &payload);
         if (!ofperr) {
             fprintf(stderr, "***decode error***");
         } else {
@@ -724,7 +726,7 @@ bundle_print_errors(struct ovs_list *errors, struct ovs_list *requests,
             LIST_FOR_EACH_CONTINUE (bmsg, list_node, requests) {
                 const struct ofp_header *oh = bmsg->data;
 
-                if (oh->xid == error->ofp_msg.xid) {
+                if (oh->xid == error_xid) {
                     ofp_msg = oh;
                     msg_len = bmsg->size;
                     break;
@@ -735,7 +737,7 @@ bundle_print_errors(struct ovs_list *errors, struct ovs_list *requests,
                       verbosity + 1);
         }
         ofpbuf_uninit(&payload);
-        free(error);
+        ofpbuf_delete(error);
     }
     fflush(stderr);
 }