This diff teaches switch(4) how to do more validations on dynamic input
field types, like: ofp_match (has N oxms), ofp_action_header (might be
followed by N actions) and ofp_instruction (might have N actions inside).
This is important because the internal switch structures reuse the ofp_match
and friends from the packet and blindly trusts it to be correct, so to be
able to do that we must ensure that we are receiving them correctly.
We need to pay special attention to the fields that these macros use:
- OFP_OXM_FOREACH;
- OFP_ACTION_FOREACH;
- OFP_FLOW_MOD_MSG_INSTRUCTION_OFFSET;
- OFP_FLOW_MOD_INSTRUCTON_FOREACH;
- OFP_I_ACTIONS_FOREACH;
- OFP_BUCKETS_FOREACH;
Other small fixes:
- Simplify OFP_FLOW_MOD_MSG_INSTRUCTION_OFFSET() macro;
- Change swofp_flow_table_add() malloc behaviour to be like all the rest
of the code (don't wait for memory);
- swofp_flow_entry_put_instructions() doesn't need a pointer to an mbuf
pointer, we only read it;
- Change the validation function parameters: we can't check for non-errors
using the value 0, because the spec actually uses it for some errors.
Instead use int pointer to get error value and use the return value only
to find out if the validation succeeded or not;
- Return more accurate error messages: some error messages were being sent
with the wrong type/code combination;
- Removed swofp_validate_flow_action_set_field() as it was incorporated
in action validation;
TODO for next diffs:
- We still need to code the validation for group messages, but since I
haven't started using it yet, I didn't touch it;
- We still need to implement validation for some specific OXM errors:
duplicated OXM, invalid wildcard, invalid value or missing prereq;
- swofp_validate_buckets() could use validate_action() with some effort;
Even though switchd(8) does packet validation (both on input and output),
we should commit this to enable other vendors/software to use the OpenBSD
switch(4) directly, otherwise we will need more effort to implement this
for switchd(8) relaying and force people to use switchd(8) needlessly.
ok?
Index: net/ofp.h
===================================================================
RCS file: /home/obsdcvs/src/sys/net/ofp.h,v
retrieving revision 1.2
diff -u -p -r1.2 ofp.h
--- net/ofp.h 30 Sep 2016 12:40:00 -0000 1.2
+++ net/ofp.h 25 Oct 2016 08:50:14 -0000
@@ -95,7 +95,7 @@ struct ofp_hello_element_versionbitmap {
/* Ports */
#define OFP_PORT_MAX 0xffffff00 /* Maximum number of physical
ports */
-#define OFP_PORT_INPUT 0xfffffff8 /* Send back to input
port */
+#define OFP_PORT_INPUT 0xfffffff8 /* Send back to input
port */
#define OFP_PORT_FLOWTABLE 0xfffffff9 /* Perform actions in flow
table */
#define OFP_PORT_NORMAL 0xfffffffa /* Let switch decide */
#define OFP_PORT_FLOOD 0xfffffffb /* All non-block ports except
input */
@@ -179,9 +179,9 @@ struct ofp_switch_features {
/* Switch capabilities */
#define OFP_SWCAP_FLOW_STATS 0x1 /* Flow statistics */
-#define OFP_SWCAP_TABLE_STATS 0x2 /* Table statistics */
-#define OFP_SWCAP_PORT_STATS 0x4 /* Port statistics */
-#define OFP_SWCAP_GROUP_STATS 0x8 /* Group statistics */
+#define OFP_SWCAP_TABLE_STATS 0x2 /* Table statistics */
+#define OFP_SWCAP_PORT_STATS 0x4 /* Port statistics */
+#define OFP_SWCAP_GROUP_STATS 0x8 /* Group statistics */
#define OFP_SWCAP_IP_REASM 0x20 /* Can reassemble IP frags */
#define OFP_SWCAP_QUEUE_STATS 0x40 /* Queue statistics */
#define OFP_SWCAP_ARP_MATCH_IP 0x80 /* Match IP addresses in ARP
pkts */
@@ -314,15 +314,15 @@ struct ofp_action_mpls_ttl {
struct ofp_action_push {
uint16_t ap_type;
uint16_t ap_len;
- uint16_t ap_ethertype;
- uint8_t pad[2];
+ uint16_t ap_ethertype;
+ uint8_t ap_pad[2];
} __packed;
struct ofp_action_pop_mpls {
uint16_t apm_type;
uint16_t apm_len;
uint16_t apm_ethertype;
- uint8_t pad[2];
+ uint8_t apm_pad[2];
} __packed;
struct ofp_action_group {
@@ -342,6 +342,12 @@ struct ofp_action_set_field {
uint16_t asf_type;
uint16_t asf_len;
uint8_t asf_field[4];
+} __packed;
+
+struct ofp_action_set_queue {
+ uint16_t asq_type;
+ uint16_t asq_len;
+ uint32_t asq_queue_id;
} __packed;
/* Packet-Out Message */
Index: net/switchofp.c
===================================================================
RCS file: /home/obsdcvs/src/sys/net/switchofp.c,v
retrieving revision 1.18
diff -u -p -r1.18 switchofp.c
--- net/switchofp.c 28 Oct 2016 09:01:49 -0000 1.18
+++ net/switchofp.c 28 Oct 2016 14:48:49 -0000
@@ -209,8 +209,13 @@ int swofp_flow_cmp_strict(struct swofp_
int swofp_flow_filter(struct swofp_flow_entry *, uint64_t, uint64_t,
uint32_t, uint32_t);
void swofp_flow_timeout(struct switch_softc *);
-int swofp_validate_flow_match(struct ofp_match *);
-int swofp_validate_flow_instruction(struct ofp_instruction *);
+int swofp_validate_oxm(struct ofp_ox_match *, int *);
+int swofp_validate_flow_match(struct ofp_match *, int *);
+int swofp_validate_flow_instruction(struct ofp_instruction *, size_t,
+ int *);
+int swofp_validate_action(struct ofp_action_header *, size_t, int *);
+int swofp_flow_entry_put_instructions(struct mbuf *,
+ struct swofp_flow_entry *, int *);
/*
* OpenFlow protocol compare oxm
@@ -388,9 +393,9 @@ for ((curr) = (head); (caddr_t)curr < ((
/*
* Get instruction offset in ofp_flow_mod
*/
-#define OFP_FLOW_MOD_MSG_INSTRUCTION_OFFSET(ofm)
\
-(offsetof(struct ofp_flow_mod, fm_match) + ntohs((ofm)->fm_match.om_length) +
\
- (((0x8 - (ntohs((ofm)->fm_match.om_length) % 0x8)) & 0x7)))
+#define OFP_FLOW_MOD_MSG_INSTRUCTION_OFFSET(ofm) \
+ (offsetof(struct ofp_flow_mod, fm_match) + \
+ OFP_ALIGN(ntohs((ofm)->fm_match.om_length)))
/*
* Instructions "FOREACH" in ofp_flow_mod. You can get header using
@@ -1238,9 +1243,10 @@ swofp_flow_table_add(struct switch_softc
if ((swft = swofp_flow_table_lookup(sc, table_id)) != NULL)
return (swft);
- new = malloc(sizeof(*new), M_DEVBUF, M_WAITOK|M_ZERO);
- new->swft_table_id = table_id;
+ if ((new = malloc(sizeof(*new), M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
+ return (NULL);
+ new->swft_table_id = table_id;
TAILQ_FOREACH(swft, &ofs->swofs_table_list, swft_table_next) {
if (table_id < swft->swft_table_id)
break;
@@ -1804,75 +1810,240 @@ swofp_ox_cmp_ether_addr(struct ofp_ox_ma
}
}
-
-/* TODO: validation for match */
int
-swofp_validate_flow_match(struct ofp_match *om)
+swofp_validate_oxm(struct ofp_ox_match *oxm, int *err)
{
- struct ofp_oxm_class *handler;
- struct ofp_ox_match *oxm;
+ struct ofp_oxm_class *handler;
+ int length, hasmask;
+ int neededlen;
- OFP_OXM_FOREACH(om, ntohs(om->om_length), oxm) {
- handler = swofp_lookup_oxm_handler(oxm);
- if (handler == NULL)
- return (OFP_ERRMATCH_BAD_FIELD);
- if (handler->oxm_match == NULL)
- return (OFP_ERRMATCH_BAD_FIELD);
+ hasmask = OFP_OXM_GET_HASMASK(oxm);
+ length = oxm->oxm_length;
+
+ handler = swofp_lookup_oxm_handler(oxm);
+ if (handler == NULL || handler->oxm_match == NULL) {
+ *err = OFP_ERRMATCH_BAD_FIELD;
+ return (-1);
+ }
+
+ neededlen = (hasmask) ?
+ (handler->oxm_len * 2) : (handler->oxm_len);
+ if (oxm->oxm_length != neededlen) {
+ *err = OFP_ERRMATCH_BAD_LEN;
+ return (-1);
}
return (0);
}
int
-swofp_validate_flow_action_set_field(struct ofp_action_set_field *oasf)
+swofp_validate_flow_match(struct ofp_match *om, int *err)
{
- struct ofp_ox_match *oxm;
- struct ofp_oxm_class *handler;
-
- oxm = (struct ofp_ox_match *)oasf->asf_field;
+ struct ofp_ox_match *oxm;
- handler = swofp_lookup_oxm_handler(oxm);
- if (handler == NULL)
- return (OFP_ERRACTION_SET_TYPE);
- if (handler->oxm_set == NULL)
- return (OFP_ERRACTION_SET_TYPE);
+ /*
+ * TODO this function is missing checks for:
+ * - OFP_ERRMATCH_BAD_TAG;
+ * - OFP_ERRMATCH_BAD_VALUE;
+ * - OFP_ERRMATCH_BAD_MASK;
+ * - OFP_ERRMATCH_BAD_PREREQ;
+ * - OFP_ERRMATCH_DUP_FIELD;
+ */
+ OFP_OXM_FOREACH(om, ntohs(om->om_length), oxm) {
+ if (swofp_validate_oxm(oxm, err))
+ return (*err);
+ }
return (0);
}
-/* TODO: validation for instruction */
int
-swofp_validate_flow_instruction(struct ofp_instruction *oi)
+swofp_validate_flow_instruction(struct ofp_instruction *oi, size_t total,
+ int *err)
{
struct ofp_action_header *oah;
struct ofp_instruction_actions *oia;
- int error = 0;
+ int ilen;
+
+ ilen = ntohs(oi->i_len);
+ /* Check for bigger than packet or smaller than header. */
+ if (ilen > total || ilen < sizeof(*oi)) {
+ *err = OFP_ERRINST_BAD_LEN;
+ return (-1);
+ }
switch (ntohs(oi->i_type)) {
case OFP_INSTRUCTION_T_GOTO_TABLE:
+ if (ilen != sizeof(struct ofp_instruction_goto_table)) {
+ *err = OFP_ERRINST_BAD_LEN;
+ return (-1);
+ }
+ break;
case OFP_INSTRUCTION_T_WRITE_META:
+ if (ilen != sizeof(struct ofp_instruction_write_metadata)) {
+ *err = OFP_ERRINST_BAD_LEN;
+ return (-1);
+ }
+ break;
case OFP_INSTRUCTION_T_METER:
- case OFP_INSTRUCTION_T_EXPERIMENTER:
+ if (ilen != sizeof(struct ofp_instruction_meter)) {
+ *err = OFP_ERRINST_BAD_LEN;
+ return (-1);
+ }
break;
+
case OFP_INSTRUCTION_T_WRITE_ACTIONS:
case OFP_INSTRUCTION_T_CLEAR_ACTIONS:
- break;
case OFP_INSTRUCTION_T_APPLY_ACTIONS:
+ if (ilen < sizeof(*oia)) {
+ *err = OFP_ERRINST_BAD_LEN;
+ return (-1);
+ }
+
oia = (struct ofp_instruction_actions *)oi;
- OFP_I_ACTIONS_FOREACH(oia, oah) {
- if (ntohs(oah->ah_type) == OFP_ACTION_SET_FIELD) {
- error = swofp_validate_flow_action_set_field(
- (struct ofp_action_set_field *)oah);
- if (error)
- break;
+
+ /* Validate actions before iterating over them. */
+ oah = (struct ofp_action_header *)
+ ((uint8_t *)oia + sizeof(*oia));
+ if (swofp_validate_action(oah, ilen - sizeof(*oia), err))
+ return (-1);
+ break;
+
+ case OFP_INSTRUCTION_T_EXPERIMENTER:
+ /* FALLTHROUGH */
+ default:
+ *err = OFP_ERRINST_UNKNOWN_INST;
+ return (-1);
+ }
+
+ return (0);
+}
+
+int
+swofp_validate_action(struct ofp_action_header *ah, size_t ahtotal, int *err)
+{
+ struct ofp_action_handler *oah;
+ struct ofp_ox_match *oxm;
+ uint8_t *dptr;
+ int ahtype, ahlen, oxmlen;
+
+ /* No actions. */
+ if (ahtotal == 0)
+ return (0);
+
+ /* Check if we have at least the first header. */
+ if (ahtotal < sizeof(*ah)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+
+ parse_next_action:
+ ahtype = ntohs(ah->ah_type);
+ ahlen = ntohs(ah->ah_len);
+ if (ahlen < sizeof(*ah) || ahlen > ahtotal) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+
+ switch (ahtype) {
+ case OFP_ACTION_OUTPUT:
+ if (ahlen != sizeof(struct ofp_action_output)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+ break;
+ case OFP_ACTION_GROUP:
+ if (ahlen != sizeof(struct ofp_action_group)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+ break;
+ case OFP_ACTION_SET_QUEUE:
+ /* TODO missing struct in ofp.h. */
+ break;
+ case OFP_ACTION_SET_MPLS_TTL:
+ if (ahlen != sizeof(struct ofp_action_mpls_ttl)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+ break;
+ case OFP_ACTION_SET_NW_TTL:
+ if (ahlen != sizeof(struct ofp_action_nw_ttl)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+ break;
+ case OFP_ACTION_COPY_TTL_OUT:
+ case OFP_ACTION_COPY_TTL_IN:
+ case OFP_ACTION_DEC_MPLS_TTL:
+ case OFP_ACTION_POP_VLAN:
+ if (ahlen != sizeof(struct ofp_action_header)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+ break;
+ case OFP_ACTION_PUSH_VLAN:
+ case OFP_ACTION_PUSH_MPLS:
+ case OFP_ACTION_PUSH_PBB:
+ if (ahlen != sizeof(struct ofp_action_push)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+ break;
+ case OFP_ACTION_POP_MPLS:
+ if (ahlen != sizeof(struct ofp_action_pop_mpls)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+ break;
+ case OFP_ACTION_SET_FIELD:
+ if (ahlen < sizeof(struct ofp_action_set_field)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+
+ oxmlen = ahlen - sizeof(struct ofp_action_set_field);
+ if (oxmlen < sizeof(*oxm)) {
+ *err = OFP_ERRACTION_LEN;
+ return (-1);
+ }
+
+ dptr = (uint8_t *)ah;
+ dptr += sizeof(struct ofp_action_set_field);
+ while (oxmlen) {
+ oxm = (struct ofp_ox_match *)dptr;
+ if (swofp_validate_oxm(oxm, err)) {
+ if (*err == OFP_ERRMATCH_BAD_LEN)
+ *err = OFP_ERRACTION_SET_LEN;
+ else
+ *err = OFP_ERRACTION_SET_TYPE;
+
+ return (-1);
}
+
+ dptr += sizeof(*oxm) + oxm->oxm_length;
+ oxmlen -= sizeof(*oxm) + oxm->oxm_length;
}
break;
+
default:
- error = EINVAL;
+ /* Unknown/unsupported action. */
+ *err = OFP_ERRACTION_TYPE;
+ return (-1);
}
- return (error);
+ oah = swofp_lookup_action_handler(ahtype);
+ /* Unknown/unsupported action. */
+ if (oah == NULL) {
+ *err = OFP_ERRACTION_TYPE;
+ return (-1);
+ }
+
+ ahtotal -= min(ahlen, ahtotal);
+ if (ahtotal)
+ goto parse_next_action;
+
+ return (0);
}
int
@@ -4565,15 +4736,15 @@ swofp_send_flow_removed(struct switch_so
* OpenFlow protocol FLOW MOD message handlers
*/
int
-swofp_flow_entry_put_instructions(struct mbuf **m,
- struct swofp_flow_entry *swfe)
+swofp_flow_entry_put_instructions(struct mbuf *m,
+ struct swofp_flow_entry *swfe, int *err)
{
struct ofp_flow_mod *ofm;
struct ofp_instruction *oi;
caddr_t inst;
- int start, len, off, error;
+ int start, len, off;
- ofm = mtod((*m), struct ofp_flow_mod *);
+ ofm = mtod(m, struct ofp_flow_mod *);
/*
* Clear instructions from flow entry. It's necessary to only modify
@@ -4585,43 +4756,72 @@ swofp_flow_entry_put_instructions(struct
start = OFP_FLOW_MOD_MSG_INSTRUCTION_OFFSET(ofm);
len = ntohs(ofm->fm_oh.oh_length) - start;
for (off = start; off < start + len; off += ntohs(oi->i_len)) {
- oi = (struct ofp_instruction *)(mtod(*m, caddr_t) + off);
-
- if ((error = swofp_validate_flow_instruction(oi)))
- goto failed;
+ oi = (struct ofp_instruction *)(mtod(m, caddr_t) + off);
+ if (swofp_validate_flow_instruction(oi,
+ len - (off - start), err))
+ return (-1);
if ((inst = malloc(ntohs(oi->i_len), M_DEVBUF,
- M_DONTWAIT|M_ZERO)) == NULL) {
- error = OFP_ERRFLOWMOD_UNKNOWN;
- goto failed;
+ M_DONTWAIT|M_ZERO)) == NULL) {
+ *err = OFP_ERRINST_EPERM;
+ return (-1);
}
memcpy(inst, oi, ntohs(oi->i_len));
switch (ntohs(oi->i_type)) {
case OFP_INSTRUCTION_T_GOTO_TABLE:
+ if (swfe->swfe_goto_table)
+ free(swfe->swfe_goto_table, M_DEVBUF,
+ ntohs(oi->i_len));
+
swfe->swfe_goto_table =
(struct ofp_instruction_goto_table *)inst;
break;
case OFP_INSTRUCTION_T_WRITE_META:
+ if (swfe->swfe_write_metadata)
+ free(swfe->swfe_write_metadata, M_DEVBUF,
+ ntohs(oi->i_len));
+
swfe->swfe_write_metadata =
(struct ofp_instruction_write_metadata *)inst;
break;
case OFP_INSTRUCTION_T_WRITE_ACTIONS:
+ if (swfe->swfe_write_actions)
+ free(swfe->swfe_write_actions, M_DEVBUF,
+ ntohs(oi->i_len));
+
swfe->swfe_write_actions =
(struct ofp_instruction_actions *)inst;
break;
case OFP_INSTRUCTION_T_APPLY_ACTIONS:
+ if (swfe->swfe_apply_actions)
+ free(swfe->swfe_apply_actions, M_DEVBUF,
+ ntohs(oi->i_len));
+
swfe->swfe_apply_actions =
(struct ofp_instruction_actions *)inst;
break;
case OFP_INSTRUCTION_T_CLEAR_ACTIONS:
+ if (swfe->swfe_clear_actions)
+ free(swfe->swfe_clear_actions, M_DEVBUF,
+ ntohs(oi->i_len));
+
swfe->swfe_clear_actions =
(struct ofp_instruction_actions *)inst;
break;
case OFP_INSTRUCTION_T_METER:
- swfe->swfe_meter = (struct ofp_instruction_meter *)inst;
+ if (swfe->swfe_meter)
+ free(swfe->swfe_meter, M_DEVBUF,
+ ntohs(oi->i_len));
+
+ swfe->swfe_meter =
+ (struct ofp_instruction_meter *)inst;
break;
case OFP_INSTRUCTION_T_EXPERIMENTER:
+ if (swfe->swfe_experimenter)
+ free(swfe->swfe_experimenter, M_DEVBUF,
+ ntohs(oi->i_len));
+
swfe->swfe_experimenter =
(struct ofp_instruction_experimenter *)inst;
break;
@@ -4632,9 +4832,6 @@ swofp_flow_entry_put_instructions(struct
}
return (0);
-
- failed:
- return (error);
}
int
@@ -4645,7 +4842,8 @@ swofp_flow_mod_cmd_add(struct switch_sof
struct ofp_match *om;
struct swofp_flow_entry *swfe, *old_swfe;
struct swofp_flow_table *swft;
- int error;
+ int error, omlen;
+ uint16_t etype = OFP_ERRTYPE_FLOW_MOD_FAILED;
oh = mtod(m, struct ofp_header *);
ofm = mtod(m, struct ofp_flow_mod *);
@@ -4662,8 +4860,30 @@ swofp_flow_mod_cmd_add(struct switch_sof
goto ofp_error;
}
- if ((swft = swofp_flow_table_lookup(sc, ofm->fm_table_id)) == NULL)
- swft = swofp_flow_table_add(sc, ofm->fm_table_id);
+ omlen = ntohs(om->om_length);
+ /*
+ * 1) ofp_match header must have at least its own size,
+ * otherwise memcpy() will fail later;
+ * 2) OXM filters can't be bigger than the packet.
+ */
+ if (omlen < sizeof(*om) &&
+ omlen >= (m->m_len - sizeof(*ofm))) {
+ etype = OFP_ERRTYPE_BAD_REQUEST;
+ error = OFP_ERRREQ_LEN;
+ goto ofp_error;
+ }
+
+ /* Validate that the OXM are in-place and correct. */
+ if (swofp_validate_flow_match(om, &error)) {
+ etype = OFP_ERRTYPE_BAD_MATCH;
+ goto ofp_error;
+ }
+
+ swft = swofp_flow_table_add(sc, ofm->fm_table_id);
+ if (swft == NULL) {
+ error = OFP_ERRFLOWMOD_TABLE_ID;
+ goto ofp_error;
+ }
if ((old_swfe = swofp_flow_search_by_table(swft, om,
ntohs(ofm->fm_priority)))) {
@@ -4687,26 +4907,25 @@ swofp_flow_mod_cmd_add(struct switch_sof
nanouptime(&swfe->swfe_installed_time);
nanouptime(&swfe->swfe_idle_time);
- if ((error = swofp_validate_flow_match(om)))
- goto ofp_error;
-
- if ((swfe->swfe_match = malloc(ntohs(om->om_length), M_DEVBUF,
+ if ((swfe->swfe_match = malloc(omlen, M_DEVBUF,
M_DONTWAIT|M_ZERO)) == NULL) {
error = OFP_ERRFLOWMOD_UNKNOWN;
goto ofp_error_free_flow;
}
- memcpy(swfe->swfe_match, om, ntohs(om->om_length));
+ memcpy(swfe->swfe_match, om, omlen);
/*
* If the ofp_match structure is empty and priority is zero, then
* this is a special flow type called table-miss which is the last
* flow to match.
*/
- if (ntohs(om->om_length) == sizeof(*om) && swfe->swfe_priority == 0)
+ if (omlen == sizeof(*om) && swfe->swfe_priority == 0)
swfe->swfe_tablemiss = 1;
- if ((error = swofp_flow_entry_put_instructions(&m, swfe)))
+ if (swofp_flow_entry_put_instructions(m, swfe, &error)) {
+ etype = OFP_ERRTYPE_BAD_INSTRUCTION;
goto ofp_error_free_flow;
+ }
if (old_swfe) {
if (!(ntohs(ofm->fm_flags) & OFP_FLOWFLAG_RESET_COUNTS)) {
@@ -4730,7 +4949,7 @@ swofp_flow_mod_cmd_add(struct switch_sof
ofp_error_free_flow:
swofp_flow_entry_free(&swfe);
ofp_error:
- swofp_send_error(sc, m, OFP_ERRTYPE_FLOW_MOD_FAILED, error);
+ swofp_send_error(sc, m, etype, error);
return (0);
}
@@ -4743,7 +4962,8 @@ swofp_flow_mod_cmd_common_modify(struct
struct ofp_match *om;
struct swofp_flow_entry *swfe;
struct swofp_flow_table *swft;
- int error;
+ int error, omlen;
+ uint16_t etype = OFP_ERRTYPE_FLOW_MOD_FAILED;
oh = mtod(m, struct ofp_header *);
ofm = mtod(m, struct ofp_flow_mod *);
@@ -4760,6 +4980,25 @@ swofp_flow_mod_cmd_common_modify(struct
goto ofp_error;
}
+ omlen = ntohs(om->om_length);
+ /*
+ * 1) ofp_match header must have at least its own size,
+ * otherwise memcpy() will fail later;
+ * 2) OXM filters can't be bigger than the packet.
+ */
+ if (omlen < sizeof(*om) &&
+ omlen >= (m->m_len - sizeof(*ofm))) {
+ etype = OFP_ERRTYPE_BAD_MATCH;
+ error = OFP_ERRMATCH_BAD_LEN;
+ goto ofp_error;
+ }
+
+ /* Validate that the OXM are in-place and correct. */
+ if (swofp_validate_flow_match(om, &error)) {
+ etype = OFP_ERRTYPE_BAD_MATCH;
+ goto ofp_error;
+ }
+
if ((swft = swofp_flow_table_lookup(sc, ofm->fm_table_id)) == NULL) {
error = OFP_ERRFLOWMOD_TABLE_ID;
goto ofp_error;
@@ -4777,7 +5016,7 @@ swofp_flow_mod_cmd_common_modify(struct
ntohl(ofm->fm_out_group)))
continue;
- if ((error = swofp_flow_entry_put_instructions(&m, swfe))) {
+ if (swofp_flow_entry_put_instructions(m, swfe, &error)) {
/*
* If error occurs in swofp_flow_entry_put_instructions,
* the flow entry might be half-way modified. So the
@@ -4785,6 +5024,7 @@ swofp_flow_mod_cmd_common_modify(struct
*/
swofp_flow_entry_delete(sc, swft, swfe,
OFP_FLOWREM_REASON_DELETE);
+ etype = OFP_ERRTYPE_BAD_INSTRUCTION;
goto ofp_error;
}
@@ -4798,7 +5038,7 @@ swofp_flow_mod_cmd_common_modify(struct
return (0);
ofp_error:
- swofp_send_error(sc, m, OFP_ERRTYPE_FLOW_MOD_FAILED, error);
+ swofp_send_error(sc, m, etype, error);
return (0);
}
@@ -4823,10 +5063,30 @@ swofp_flow_mod_cmd_common_delete(struct
struct ofp_flow_mod *ofm;
struct ofp_match *om;
struct swofp_flow_table *swft;
+ int error, omlen;
+ uint16_t etype = OFP_ERRTYPE_FLOW_MOD_FAILED;
ofm = (struct ofp_flow_mod *)(mtod(m, caddr_t));
om = &ofm->fm_match;
+ omlen = ntohs(om->om_length);
+ /*
+ * 1) ofp_match header must have at least its own size,
+ * otherwise memcpy() will fail later;
+ * 2) OXM filters can't be bigger than the packet.
+ */
+ if (omlen < sizeof(*om) &&
+ omlen >= (m->m_len - sizeof(*ofm))) {
+ error = OFP_ERRFLOWMOD_UNKNOWN;
+ goto ofp_error;
+ }
+
+ /* Validate that the OXM are in-place and correct. */
+ if (swofp_validate_flow_match(om, &error)) {
+ etype = OFP_ERRTYPE_BAD_MATCH;
+ goto ofp_error;
+ }
+
TAILQ_FOREACH(swft, &ofs->swofs_table_list, swft_table_next) {
if ((ofm->fm_table_id != OFP_TABLE_ID_ALL) &&
(ofm->fm_table_id != swft->swft_table_id))
@@ -4842,6 +5102,10 @@ swofp_flow_mod_cmd_common_delete(struct
m_freem(m);
return (0);
+
+ ofp_error:
+ swofp_send_error(sc, m, etype, error);
+ return (-1);
}
int
@@ -5067,7 +5331,7 @@ swofp_recv_packet_out(struct switch_soft
struct ofp_packet_out *pout;
struct ofp_action_header *ah;
struct mbuf *mc = NULL, *mcn;
- int al_start, al_len, off;
+ int al_start, al_len, off, error;
struct switch_flow_classify swfcl = {};
struct swofp_pipline_desc swpld = { .swpld_swfcl = &swfcl };
@@ -5084,6 +5348,15 @@ swofp_recv_packet_out(struct switch_soft
pout = mtod(m, struct ofp_packet_out *);
al_start = offsetof(struct ofp_packet_out, pout_actions);
+
+ /* Validate actions before anything else. */
+ ah = (struct ofp_action_header *)
+ ((uint8_t *)pout + sizeof(*pout));
+ if (swofp_validate_action(ah, al_len, &error)) {
+ swofp_send_error(sc, m, OFP_ERRTYPE_BAD_REQUEST, error);
+ return (EINVAL);
+ }
+
if (pout->pout_buffer_id == OFP_PKTOUT_NO_BUFFER) {
/*
* It's not necessary to deep copy at here because it's done
