On Wed, Feb 27, 2013 at 04:12:16PM +0900, Simon Horman wrote:
> ofpbuf_put_* may reallocate the underlying buffer of the ofpbuf and
> thus writing data after a ofpbuf_put_* call must write to memory
> relative to the pointer returned by the call.
>
> Prior to this change the length and trailing value would not be written to
> the set_field action if ofpbuf_put_* may reallocated the underlying buffer.
>
> Also make use of ofpbuf_put_zero() to avoid calling memset() directly.
>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
Good catch!
I think that we don't really need to reload oasf at all, because we
can just initialize oasf->len earlier and then not refer back to oasf
again after the ofpbuf_put(). That seems a little nicer. So how
about this:
--8<--------------------------cut here-------------------------->8--
From: Simon Horman <ho...@verge.net.au>
Date: Wed, 27 Feb 2013 16:12:16 +0900
Subject: [PATCH] nx-match: Correct writing of value and length in
set_field_to_ofast()
ofpbuf_put_* may reallocate the underlying buffer of the ofpbuf and
thus writing data after a ofpbuf_put_* call must write to memory
relative to the pointer returned by the call.
Prior to this change the length and trailing value would not be written to
the set_field action if ofpbuf_put_* may reallocated the underlying buffer.
Also make use of ofpbuf_put_zero() to avoid calling memset() directly.
Signed-off-by: Simon Horman <ho...@verge.net.au>
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
lib/nx-match.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 4ff516e..e5545de 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1217,22 +1217,19 @@ set_field_to_ofast(const struct ofpact_reg_load *load,
struct ofpbuf *openflow)
{
const struct mf_field *mf = load->dst.field;
+ uint16_t padded_value_len = ROUND_UP(mf->n_bytes, 8);
struct ofp12_action_set_field *oasf;
- uint16_t padded_value_len;
-
- oasf = ofputil_put_OFPAT12_SET_FIELD(openflow);
- oasf->dst = htonl(mf->oxm_header);
+ char *value;
/* Set field is the only action of variable length (so far),
* so handling the variable length portion is open-coded here */
- padded_value_len = ROUND_UP(mf->n_bytes, 8);
- ofpbuf_put_uninit(openflow, padded_value_len);
+ oasf = ofputil_put_OFPAT12_SET_FIELD(openflow);
+ oasf->dst = htonl(mf->oxm_header);
oasf->len = htons(ntohs(oasf->len) + padded_value_len);
- memset(oasf + 1, 0, padded_value_len);
+ value = ofpbuf_put_zeros(openflow, padded_value_len);
bitwise_copy(&load->subvalue, sizeof load->subvalue, load->dst.ofs,
- oasf + 1, mf->n_bytes, load->dst.ofs, load->dst.n_bits);
- return;
+ value, mf->n_bytes, load->dst.ofs, load->dst.n_bits);
}
void
--
1.7.2.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
