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

Murphy McCauley murphy.mccauley at gmail.com
Tue Oct 18 22:00:36 UTC 2011


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 <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;





More information about the discuss mailing list