From 7ac27a044457b560836f6dc9c11443ba7e02045d Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 1 Jun 2015 18:07:39 -0700 Subject: [PATCH] bundles: Validate bundled messages. OpenFlow bundle messages should be decoded and validated at the time they are added to the bundle. This commit does this for flow mod and port mod messages. Signed-off-by: Jarno Rajahalme Acked-by: Ben Pfaff --- lib/ofp-print.c | 4 ++-- lib/ofp-util.c | 10 +++++++--- lib/ofp-util.h | 7 +++++-- ofproto/bundles.c | 36 ++++++++++++++-------------------- ofproto/bundles.h | 49 ++++++++++++++++++++++++++++++++++++++--------- ofproto/ofproto.c | 41 +++++++++++++++++++++++++++++++++++++-- 6 files changed, 107 insertions(+), 40 deletions(-) diff --git a/lib/ofp-print.c b/lib/ofp-print.c index cec074f1e..d773dca4f 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -2657,7 +2657,7 @@ ofp_print_bundle_add(struct ds *s, const struct ofp_header *oh, int verbosity) struct ofputil_bundle_add_msg badd; char *msg; - error = ofputil_decode_bundle_add(oh, &badd); + error = ofputil_decode_bundle_add(oh, &badd, NULL); if (error) { ofp_print_error(s, error); return; diff --git a/lib/ofp-util.c b/lib/ofp-util.c index e62c584fd..17a0c412a 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -8852,7 +8852,8 @@ ofputil_is_bundlable(enum ofptype type) enum ofperr ofputil_decode_bundle_add(const struct ofp_header *oh, - struct ofputil_bundle_add_msg *msg) + struct ofputil_bundle_add_msg *msg, + enum ofptype *type_ptr) { const struct ofp14_bundle_ctrl_msg *m; struct ofpbuf b; @@ -8879,14 +8880,17 @@ ofputil_decode_bundle_add(const struct ofp_header *oh, } /* Reject unbundlable messages. */ - error = ofptype_decode(&type, msg->msg); + if (!type_ptr) { + type_ptr = &type; + } + error = ofptype_decode(type_ptr, msg->msg); if (error) { VLOG_WARN_RL(&bad_ofmsg_rl, "OFPT14_BUNDLE_ADD_MESSAGE contained " "message is unparsable (%s)", ofperr_get_name(error)); return OFPERR_OFPBFC_MSG_UNSUP; /* 'error' would be confusing. */ } - if (!ofputil_is_bundlable(type)) { + if (!ofputil_is_bundlable(*type_ptr)) { return OFPERR_OFPBFC_MSG_UNSUP; } diff --git a/lib/ofp-util.h b/lib/ofp-util.h index ee3f1bed6..efb5b18f0 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1109,6 +1109,8 @@ struct ofputil_bundle_add_msg { const struct ofp_header *msg; }; +enum ofptype; + enum ofperr ofputil_decode_bundle_ctrl(const struct ofp_header *, struct ofputil_bundle_ctrl_msg *); @@ -1119,5 +1121,6 @@ struct ofpbuf *ofputil_encode_bundle_add(enum ofp_version ofp_version, struct ofputil_bundle_add_msg *msg); enum ofperr ofputil_decode_bundle_add(const struct ofp_header *, - struct ofputil_bundle_add_msg *); + struct ofputil_bundle_add_msg *, + enum ofptype *type); #endif /* ofp-util.h */ diff --git a/ofproto/bundles.c b/ofproto/bundles.c index c40909150..f6ad6086e 100644 --- a/ofproto/bundles.c +++ b/ofproto/bundles.c @@ -1,7 +1,7 @@ /* * Copyright (c) 2013, 2014 Alexandru Copot , with support from IXIA. * Copyright (c) 2013, 2014 Daniel Baluta - * Copyright (c) 2014 Nicira, Inc. + * Copyright (c) 2014, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,11 +57,6 @@ struct ofp_bundle { struct ovs_list msg_list; }; -struct bundle_message { - struct ofp_header *msg; - struct ovs_list node; /* Element in 'struct ofp_bundles's msg_list */ -}; - static uint32_t bundle_hash(uint32_t id) { @@ -98,20 +93,19 @@ ofp_bundle_create(uint32_t id, uint16_t flags) } static void -ofp_bundle_remove(struct ofconn *ofconn, struct ofp_bundle *item) +ofp_bundle_remove(struct ofconn *ofconn, struct ofp_bundle *bundle) { - struct bundle_message *msg; + struct ofp_bundle_entry *msg; struct hmap *bundles; - LIST_FOR_EACH_POP (msg, node, &item->msg_list) { - free(msg->msg); - free(msg); + LIST_FOR_EACH_POP (msg, node, &bundle->msg_list) { + ofp_bundle_entry_free(msg); } bundles = ofconn_get_bundles(ofconn); - hmap_remove(bundles, &item->node); + hmap_remove(bundles, &bundle->node); - free(item); + free(bundle); } void @@ -187,7 +181,7 @@ ofp_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags) struct hmap *bundles; struct ofp_bundle *bundle; enum ofperr error = 0; - struct bundle_message *msg; + struct ofp_bundle_entry *msg; bundles = ofconn_get_bundles(ofconn); bundle = ofp_bundle_find(bundles, id); @@ -227,31 +221,29 @@ ofp_bundle_discard(struct ofconn *ofconn, uint32_t id) } enum ofperr -ofp_bundle_add_message(struct ofconn *ofconn, struct ofputil_bundle_add_msg *badd) +ofp_bundle_add_message(struct ofconn *ofconn, uint32_t id, uint16_t flags, + struct ofp_bundle_entry *bmsg) { struct hmap *bundles; struct ofp_bundle *bundle; - struct bundle_message *bmsg; bundles = ofconn_get_bundles(ofconn); - bundle = ofp_bundle_find(bundles, badd->bundle_id); + bundle = ofp_bundle_find(bundles, id); if (!bundle) { - bundle = ofp_bundle_create(badd->bundle_id, badd->flags); + bundle = ofp_bundle_create(id, flags); bundle->state = BS_OPEN; bundles = ofconn_get_bundles(ofconn); - hmap_insert(bundles, &bundle->node, bundle_hash(badd->bundle_id)); + hmap_insert(bundles, &bundle->node, bundle_hash(id)); } else if (bundle->state == BS_CLOSED) { ofp_bundle_remove(ofconn, bundle); return OFPERR_OFPBFC_BUNDLE_CLOSED; - } else if (badd->flags != bundle->flags) { + } else if (flags != bundle->flags) { ofp_bundle_remove(ofconn, bundle); return OFPERR_OFPBFC_BAD_FLAGS; } - bmsg = xmalloc(sizeof *bmsg); - bmsg->msg = xmemdup(badd->msg, ntohs(badd->msg->length)); list_push_back(&bundle->msg_list, &bmsg->node); return 0; } diff --git a/ofproto/bundles.h b/ofproto/bundles.h index 9a6dfa5a1..c8ce5c985 100644 --- a/ofproto/bundles.h +++ b/ofproto/bundles.h @@ -1,6 +1,7 @@ /* * Copyright (c) 2013, 2014 Alexandru Copot , with support from IXIA. * Copyright (c) 2013, 2014 Daniel Baluta + * Copyright (c) 2014, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,27 +21,57 @@ #include -#include "ofp-msgs.h" #include "connmgr.h" +#include "list.h" +#include "ofp-msgs.h" #include "ofp-util.h" #ifdef __cplusplus extern "C" { #endif +struct ofp_bundle_entry { + struct ovs_list node; + ovs_be32 xid; /* For error returns. */ + enum ofptype type; /* OFPTYPE_FLOW_MOD or OFPTYPE_PORT_MOD. */ + union { + struct ofputil_flow_mod fm; /* 'fm.ofpacts' must be malloced. */ + struct ofputil_port_mod pm; + }; +}; -enum ofperr ofp_bundle_open(struct ofconn *ofconn, uint32_t id, uint16_t flags); - -enum ofperr ofp_bundle_close(struct ofconn *ofconn, uint32_t id, uint16_t flags); +static inline struct ofp_bundle_entry *ofp_bundle_entry_alloc( + enum ofptype type, ovs_be32 xid); +static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *); -enum ofperr ofp_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags); +enum ofperr ofp_bundle_open(struct ofconn *, uint32_t id, uint16_t flags); +enum ofperr ofp_bundle_close(struct ofconn *, uint32_t id, uint16_t flags); +enum ofperr ofp_bundle_commit(struct ofconn *, uint32_t id, uint16_t flags); +enum ofperr ofp_bundle_discard(struct ofconn *, uint32_t id); +enum ofperr ofp_bundle_add_message(struct ofconn *, uint32_t id, + uint16_t flags, struct ofp_bundle_entry *); +void ofp_bundle_remove_all(struct ofconn *); + +static inline struct ofp_bundle_entry * +ofp_bundle_entry_alloc(enum ofptype type, ovs_be32 xid) +{ + struct ofp_bundle_entry *entry = xmalloc(sizeof *entry); -enum ofperr ofp_bundle_discard(struct ofconn *ofconn, uint32_t id); + entry->xid = xid; + entry->type = type; -enum ofperr ofp_bundle_add_message(struct ofconn *ofconn, - struct ofputil_bundle_add_msg *badd); + return entry; +} -void ofp_bundle_remove_all(struct ofconn *ofconn); +static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *entry) +{ + if (entry) { + if (entry->type == OFPTYPE_FLOW_MOD) { + free(entry->fm.ofpacts); + } + free(entry); + } +} #ifdef __cplusplus } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 9c4e97d27..0a8c82af5 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -6357,20 +6357,57 @@ handle_bundle_control(struct ofconn *ofconn, const struct ofp_header *oh) static enum ofperr handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh) { + struct ofproto *ofproto = ofconn_get_ofproto(ofconn); enum ofperr error; struct ofputil_bundle_add_msg badd; + struct ofp_bundle_entry *bmsg; + enum ofptype type; error = reject_slave_controller(ofconn); if (error) { return error; } - error = ofputil_decode_bundle_add(oh, &badd); + error = ofputil_decode_bundle_add(oh, &badd, &type); if (error) { return error; } - return ofp_bundle_add_message(ofconn, &badd); + bmsg = ofp_bundle_entry_alloc(type, badd.msg->xid); + + if (type == OFPTYPE_PORT_MOD) { + error = ofputil_decode_port_mod(badd.msg, &bmsg->pm, false); + } else if (type == OFPTYPE_FLOW_MOD) { + struct ofpbuf ofpacts; + uint64_t ofpacts_stub[1024 / 8]; + + ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub); + error = ofputil_decode_flow_mod(&bmsg->fm, badd.msg, + ofconn_get_protocol(ofconn), + &ofpacts, + u16_to_ofp(ofproto->max_ports), + ofproto->n_tables); + /* Move actions to heap. */ + bmsg->fm.ofpacts = ofpbuf_steal_data(&ofpacts); + + if (!error && bmsg->fm.ofpacts_len) { + error = ofproto_check_ofpacts(ofproto, bmsg->fm.ofpacts, + bmsg->fm.ofpacts_len); + } + } else { + OVS_NOT_REACHED(); + } + + if (!error) { + error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags, + bmsg); + } + + if (error) { + ofp_bundle_entry_free(bmsg); + } + + return error; } static enum ofperr -- 2.20.1