The controller action had a wrong wrong userdata_len due to missing substraction of previous data len. The pointer to oc was not moved correctly so it could in theory point to wrong memory. In order to fix that expose encode_start_controller_op and encode_finish_controller_op which are helper methods to define the controller action. Use those instead of manually creating the action. At the same time use stack buffer for the ofpbuf which should be large enough to hold related data without any heap allocations.
Fixes: e52c2451 ("physical.c: Fix bug of wrong use in vm migration") Signed-off-by: Ales Musil <amu...@redhat.com> --- controller/physical.c | 19 ++++++------------- include/ovn/actions.h | 4 ++++ lib/actions.c | 4 ++-- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/controller/physical.c b/controller/physical.c index 552837bcd..bc8c304fe 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -992,8 +992,8 @@ setup_rarp_activation_strategy(const struct sbrec_port_binding *binding, struct ovn_desired_flow_table *flow_table) { struct match match = MATCH_CATCHALL_INITIALIZER; - struct ofpbuf ofpacts; - ofpbuf_init(&ofpacts, 0); + uint64_t stub[1024 / 8]; + struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub); /* Unblock the port on ingress RARP. */ match_set_dl_type(&match, htons(ETH_TYPE_RARP)); @@ -1001,18 +1001,11 @@ setup_rarp_activation_strategy(const struct sbrec_port_binding *binding, load_logical_ingress_metadata(binding, zone_ids, &ofpacts); - struct ofpact_controller *oc = ofpact_put_CONTROLLER(&ofpacts); - oc->max_len = UINT16_MAX; - oc->reason = OFPR_ACTION; - - struct action_header ah = { - .opcode = htonl(ACTION_OPCODE_ACTIVATION_STRATEGY_RARP) - }; - ofpbuf_put(&ofpacts, &ah, sizeof ah); + uint32_t ofs = + encode_start_controller_op(ACTION_OPCODE_ACTIVATION_STRATEGY_RARP, + false, NX_CTLR_NO_METER, &ofpacts); + encode_finish_controller_op(ofs, &ofpacts); - ofpacts.header = oc; - oc->userdata_len = ofpacts.size - (sizeof *oc); - ofpact_finish_CONTROLLER(&ofpacts, &oc); put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts); ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 1010, diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 33c319f1c..cbcf4130c 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -829,4 +829,8 @@ void ovnacts_free(struct ovnact[], size_t ovnacts_len); char *ovnact_op_to_string(uint32_t); int encode_ra_dnssl_opt(char *data, char *buf, int buf_len); +size_t encode_start_controller_op(enum action_opcode opcode, bool pause, + uint32_t meter_id, struct ofpbuf *ofpacts); +void encode_finish_controller_op(size_t ofs, struct ofpbuf *ofpacts); + #endif /* ovn/actions.h */ diff --git a/lib/actions.c b/lib/actions.c index aab044306..5c70ef411 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -78,7 +78,7 @@ ovnact_init(struct ovnact *ovnact, enum ovnact_type type, size_t len) ovnact->len = len; } -static size_t +size_t encode_start_controller_op(enum action_opcode opcode, bool pause, uint32_t meter_id, struct ofpbuf *ofpacts) { @@ -99,7 +99,7 @@ encode_start_controller_op(enum action_opcode opcode, bool pause, return ofs; } -static void +void encode_finish_controller_op(size_t ofs, struct ofpbuf *ofpacts) { struct ofpact_controller *oc = ofpbuf_at_assert(ofpacts, ofs, sizeof *oc); -- 2.37.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev