This diff is a part of the bigger diff to add more input validations to
the switch(4) OpenFlow protocol parser.
In this diff we reworked the swofp_flow_entry_put_instructions() function
with the following changes:
- Avoid leaking memory on repeated instructions. It is not possible to
use the same instruction more than once, however the spec doesn't
specify any errors for this ocasion. (OpenFlow 1.3.5 spec page 26);
- Apply the same error return tecnique for this function;
- Remove old goto label;
- Fix some whitespace/tab issue;
ok?
Index: net/switchofp.c
===================================================================
RCS file: /cvs/src/sys/net/switchofp.c,v
retrieving revision 1.23
diff -u -p -r1.23 switchofp.c
--- net/switchofp.c 31 Oct 2016 08:06:27 -0000 1.23
+++ net/switchofp.c 31 Oct 2016 08:24:43 -0000
@@ -195,7 +195,7 @@ int swofp_validate_buckets(struct switc
* Flow entry
*/
int swofp_flow_entry_put_instructions(struct mbuf *,
- struct swofp_flow_entry *);
+ struct swofp_flow_entry *, int *error);
void swofp_flow_entry_instruction_free(struct swofp_flow_entry *);
void swofp_flow_entry_free(struct swofp_flow_entry **);
void swofp_flow_entry_add(struct switch_softc *, struct swofp_flow_table *,
@@ -4570,12 +4570,12 @@ swofp_send_flow_removed(struct switch_so
*/
int
swofp_flow_entry_put_instructions(struct mbuf *m,
- struct swofp_flow_entry *swfe)
+ struct swofp_flow_entry *swfe, int *error)
{
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 *);
@@ -4591,41 +4591,69 @@ swofp_flow_entry_put_instructions(struct
for (off = start; off < start + len; off += ntohs(oi->i_len)) {
oi = (struct ofp_instruction *)(mtod(m, caddr_t) + off);
- if (swofp_validate_flow_instruction(oi, &error))
- goto failed;
+ if (swofp_validate_flow_instruction(oi, error))
+ 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) {
+ *error = OFP_ERRFLOWMOD_UNKNOWN;
+ 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:
+ 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;
@@ -4636,9 +4664,6 @@ swofp_flow_entry_put_instructions(struct
}
return (0);
-
- failed:
- return (error);
}
int
@@ -4712,7 +4737,7 @@ swofp_flow_mod_cmd_add(struct switch_sof
if (ntohs(om->om_length) == 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;
}
@@ -4787,7 +4812,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
