[ovs-dev] [PATCH] ofp-print: Fix misaligned data access in ofp_print_error_msg().
Jarno Rajahalme
jrajahalme at nicira.com
Sat Apr 5 17:33:02 UTC 2014
(Did not check if there are other users of ofperr_decode_msg(), maybe it would have been a good idea to change the signature in some way to cause at least a warning in case some other users exist out there.)
Looks good to me,
Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
> On Apr 4, 2014, at 7:30 PM, Ben Pfaff <blp at nicira.com> wrote:
>
> The body of an OpenFlow error message often contains an inner OpenFlow
> message, and when it does, the inner message starts at an odd multiple of 4
> bytes from the beginning of the outer message. That means that, on RISC
> systems, accessing the inner message directly causes a bus error. This
> commit fixes the problem in a way that should make it difficult to recur.
>
> This fixes the failure of tests 643, 645, and 651 on sparc seen here:
> https://buildd.debian.org/status/fetch.php?pkg=openvswitch&arch=sparc&ver=2.1.0%2Bgit20140325-1&stamp=1396438624
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> lib/ofp-errors.c | 11 +++++++----
> lib/ofp-print.c | 1 +
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/lib/ofp-errors.c b/lib/ofp-errors.c
> index d1e4dbf..bd4e43a 100644
> --- a/lib/ofp-errors.c
> +++ b/lib/ofp-errors.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2012, 2013, 2014 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -272,8 +272,10 @@ ofperr_get_code(enum ofperr error, enum ofp_version version)
> /* Tries to decode 'oh', which should be an OpenFlow OFPT_ERROR message.
> * Returns an OFPERR_* constant on success, 0 on failure.
> *
> - * If 'payload' is nonnull, on success '*payload' is initialized to the
> - * error's payload, and on failure it is cleared. */
> + * If 'payload' is nonnull, on success '*payload' is initialized with a copy of
> + * the error's payload (copying is required because the payload is not properly
> + * aligned). The caller must free the payload (with ofpbuf_uninit()) when it
> + * is no longer needed. On failure, '*payload' is cleared. */
> enum ofperr
> ofperr_decode_msg(const struct ofp_header *oh, struct ofpbuf *payload)
> {
> @@ -323,7 +325,8 @@ ofperr_decode_msg(const struct ofp_header *oh, struct ofpbuf *payload)
> /* Translate the error type and code into an ofperr. */
> error = ofperr_decode(oh->version, vendor, type, code);
> if (error && payload) {
> - ofpbuf_use_const(payload, ofpbuf_data(&b), ofpbuf_size(&b));
> + ofpbuf_init(payload, ofpbuf_size(&b));
> + ofpbuf_push(payload, ofpbuf_data(&b), ofpbuf_size(&b));
> }
> return error;
> }
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index c8c331e..e0e565c 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1367,6 +1367,7 @@ ofp_print_error_msg(struct ds *string, const struct ofp_header *oh)
> ds_put_cstr(string, s);
> free(s);
> }
> + ofpbuf_uninit(&payload);
> }
>
> static void
> --
> 1.8.5.3
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list