mirror of
https://github.com/openvswitch/ovs
synced 2025-09-01 14:55:18 +00:00
ofp-actions: enforce valid range for table_id in goto_table instruction
Found a bug that OVS allows goto_table_id to be smaller than (or equal to) the current table id where the flow resides. It potentially creates an infinite loop when composing actions for a packet. To fix it, we just let OVS returns an error message to prevent such flow to be programmed. Signed-off-by: Jing Ai <jinga@google.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
This commit is contained in:
2
AUTHORS
2
AUTHORS
@@ -46,6 +46,7 @@ Jarno Rajahalme jarno.rajahalme@nsn.com
|
||||
Jean Tourrilhes jt@hpl.hp.com
|
||||
Jeremy Stribling strib@nicira.com
|
||||
Jesse Gross jesse@nicira.com
|
||||
Jing Ai jinga@google.com
|
||||
Joe Perches joe@perches.com
|
||||
Joe Stringer joe@wand.net.nz
|
||||
Jun Nakajima jun.nakajima@intel.com
|
||||
@@ -154,7 +155,6 @@ Jari Sundell sundell.software@gmail.com
|
||||
Jed Daniels openvswitch@jeddaniels.com
|
||||
Jeff Merrick jmerrick@vmware.com
|
||||
Jeongkeun Lee jklee@hp.com
|
||||
Jing Ai ai_jing2000@hotmail.com
|
||||
Joan Cirer joan@ev0.net
|
||||
John Galgay john@galgay.net
|
||||
Kevin Mancuso kevin.mancuso@rackspace.com
|
||||
|
@@ -1052,6 +1052,7 @@ ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,
|
||||
enum ofperr
|
||||
ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
|
||||
unsigned int instructions_len,
|
||||
uint8_t table_id,
|
||||
struct ofpbuf *ofpacts)
|
||||
{
|
||||
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
|
||||
@@ -1119,6 +1120,10 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
|
||||
|
||||
oigt = instruction_get_OFPIT11_GOTO_TABLE(
|
||||
insts[OVSINST_OFPIT11_GOTO_TABLE]);
|
||||
if (table_id >= oigt->table_id) {
|
||||
error = OFPERR_OFPBRC_BAD_TABLE_ID;
|
||||
goto exit;
|
||||
}
|
||||
ogt = ofpact_put_GOTO_TABLE(ofpacts);
|
||||
ogt->table_id = oigt->table_id;
|
||||
}
|
||||
|
@@ -490,6 +490,7 @@ enum ofperr ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,
|
||||
struct ofpbuf *ofpacts);
|
||||
enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
|
||||
unsigned int instructions_len,
|
||||
uint8_t table_id,
|
||||
struct ofpbuf *ofpacts);
|
||||
enum ofperr ofpacts_check(const struct ofpact[], size_t ofpacts_len,
|
||||
const struct flow *, int max_ports);
|
||||
|
@@ -1502,7 +1502,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
|
||||
return error;
|
||||
}
|
||||
|
||||
error = ofpacts_pull_openflow11_instructions(&b, b.size, ofpacts);
|
||||
error = ofpacts_pull_openflow11_instructions(&b, b.size, ofm->table_id,
|
||||
ofpacts);
|
||||
if (error) {
|
||||
return error;
|
||||
}
|
||||
@@ -2014,7 +2015,8 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
|
||||
}
|
||||
|
||||
if (ofpacts_pull_openflow11_instructions(msg, length - sizeof *ofs -
|
||||
padded_match_len, ofpacts)) {
|
||||
padded_match_len,
|
||||
ofs->table_id, ofpacts)) {
|
||||
VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
|
||||
return EINVAL;
|
||||
}
|
||||
|
@@ -356,6 +356,10 @@ dnl Goto-Table 1 instruction non-zero padding
|
||||
# 7: 01 -> 00
|
||||
0001 0008 01 000001
|
||||
|
||||
dnl Goto-Table 1 instruction go back to the previous table.
|
||||
# bad OF1.1 instructions: OFPBRC_BAD_TABLE_ID
|
||||
2,0001 0008 01 000000
|
||||
|
||||
dnl Goto-Table 1
|
||||
# actions=goto_table:1
|
||||
0001 0008 01 000000
|
||||
|
@@ -2607,17 +2607,28 @@ ofctl_parse_ofp11_instructions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
|
||||
enum ofperr error;
|
||||
size_t size;
|
||||
struct ds s;
|
||||
const char *table_id;
|
||||
char *instructions;
|
||||
|
||||
/* Parse table_id separated with the follow-up instructions by ",", if
|
||||
* any. */
|
||||
instructions = ds_cstr(&in);
|
||||
table_id = NULL;
|
||||
if (strstr(instructions, ",")) {
|
||||
table_id = strsep(&instructions, ",");
|
||||
}
|
||||
|
||||
/* Parse hex bytes. */
|
||||
ofpbuf_init(&of11_in, 0);
|
||||
if (ofpbuf_put_hex(&of11_in, ds_cstr(&in), NULL)[0] != '\0') {
|
||||
if (ofpbuf_put_hex(&of11_in, instructions, NULL)[0] != '\0') {
|
||||
ovs_fatal(0, "Trailing garbage in hex data");
|
||||
}
|
||||
|
||||
/* Convert to ofpacts. */
|
||||
ofpbuf_init(&ofpacts, 0);
|
||||
size = of11_in.size;
|
||||
error = ofpacts_pull_openflow11_instructions(&of11_in, of11_in.size,
|
||||
error = ofpacts_pull_openflow11_instructions(
|
||||
&of11_in, of11_in.size, table_id ? atoi(table_id) : 0,
|
||||
&ofpacts);
|
||||
if (error) {
|
||||
printf("bad OF1.1 instructions: %s\n\n", ofperr_get_name(error));
|
||||
|
Reference in New Issue
Block a user