Yeah, my temporary patch does the same thing (though I explicitly called push_uninit and memcpy because I didn't notice ofpbuf_push). I wasn't sure if I was missing some better solution.
That's a good thought on locating other places. I probably don't have time to do it immediately, but something to keep in mind if this continues to bite me. :) Thanks. -- Murphy On Tue, 2011-10-18 at 14:01 -0700, Ben Pfaff wrote: > 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 <[email protected]> > 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 <[email protected]> > --- > 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 [email protected] > Luiz Henrique Ozaki [email protected] > Michael Hu [email protected] > Michael Mao [email protected] > +Murphy McCauley [email protected] > Mikael Doverhag [email protected] > Niklas Andersson [email protected] > Pankaj Thakkar [email protected] > 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; _______________________________________________ discuss mailing list [email protected] http://openvswitch.org/mailman/listinfo/discuss
