[ovs-discuss] Bad OpenFlow buffer IDs, etc.

[Ben Pfaff]

On Mon, Oct 17, 2011 at 12:38:17PM -0700, Murphy McCauley wrote:
> The problem with buffer IDs is in ofputil_encode_packet_in() which
> writes to an unaligned ofp_packet_in pointer.
> 
> Any thoughts on a good way to fix this, or how to locate other places
> where the same thing may be happening (there's no warning or anything)?

The fix itself is easy.  I've appended my first thought at how to do
it.  Please test it and let me know the results.

As for how to locate other places, I don't have good ideas.  Probably,
testing on platforms that signal misaligned accesses (such as SPARC)
instead of on platforms that rotate bytes on misaligned accesses (only
ARM, as far as I know), is a good idea.  Static analysis is better,
but I don't have a good way to do it.

--8<--------------------------cut here-------------------------->8--

>From 9d734d8341d2f2636d7402145f6934544c8f7e1c Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp at nicira.com>
Date: Tue, 18 Oct 2011 13:58:21 -0700
Subject: [PATCH] ofp-util: Avoid misaligned memory access in
 ofputil_encode_packet_in().

Reported-by: Murphy McCauley <murphy.mccauley at gmail.com>
---
 AUTHORS        |    1 +
 lib/ofp-util.c |   17 +++++++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 3229f34..e00feea 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -84,6 +84,7 @@ Krishna Miriyala        krishna at nicira.com
 Luiz Henrique Ozaki     luiz.ozaki at gmail.com
 Michael Hu              mhu at nicira.com
 Michael Mao             mmao at nicira.com
+Murphy McCauley         murphy.mccauley at gmail.com
 Mikael Doverhag         mdoverhag at nicira.com
 Niklas Andersson        nandersson at nicira.com
 Pankaj Thakkar          thakkar at nicira.com
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index b46219a..0930196 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1449,7 +1449,7 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
                         struct ofpbuf *rw_packet)
 {
     int total_len = pin->packet->size;
-    struct ofp_packet_in *opi;
+    struct ofp_packet_in opi;
 
     if (rw_packet) {
         if (pin->send_len < rw_packet->size) {
@@ -1462,13 +1462,14 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
     }
 
     /* Add OFPT_PACKET_IN. */
-    opi = ofpbuf_push_zeros(rw_packet, offsetof(struct ofp_packet_in, data));
-    opi->header.version = OFP_VERSION;
-    opi->header.type = OFPT_PACKET_IN;
-    opi->total_len = htons(total_len);
-    opi->in_port = htons(pin->in_port);
-    opi->reason = pin->reason;
-    opi->buffer_id = htonl(pin->buffer_id);
+    memset(&opi, 0, sizeof opi);
+    opi.header.version = OFP_VERSION;
+    opi.header.type = OFPT_PACKET_IN;
+    opi.total_len = htons(total_len);
+    opi.in_port = htons(pin->in_port);
+    opi.reason = pin->reason;
+    opi.buffer_id = htonl(pin->buffer_id);
+    ofpbuf_push(rw_packet, &opi, offsetof(struct ofp_packet_in, data));
     update_openflow_length(rw_packet);
 
     return rw_packet;
-- 
1.7.4.4