From 4ee2f4c180ec32946eb1d332a3afb151d7e6d832 Mon Sep 17 00:00:00 2001 From: "Andrey V. Elsukov" Date: Fri, 12 Jul 2019 09:48:42 +0000 Subject: Correctly truncate the rule in case when it has several action opcodes. It is possible, that opcode at the ACTION_PTR() location is not real action, but action modificator like "log", "tag" etc. In this case we need to check for each opcode in the loop to find O_EXTERNAL_ACTION. Obtained from: Yandex LLC MFC after: 1 week Sponsored by: Yandex LLC --- sys/netpfil/ipfw/ip_fw_eaction.c | 44 +++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 14 deletions(-) (limited to 'sys/netpfil/ipfw') diff --git a/sys/netpfil/ipfw/ip_fw_eaction.c b/sys/netpfil/ipfw/ip_fw_eaction.c index 1cb2f812936c..33cc450b0046 100644 --- a/sys/netpfil/ipfw/ip_fw_eaction.c +++ b/sys/netpfil/ipfw/ip_fw_eaction.c @@ -377,35 +377,51 @@ ipfw_reset_eaction(struct ip_fw_chain *ch, struct ip_fw *rule, uint16_t eaction_id, uint16_t default_id, uint16_t instance_id) { ipfw_insn *cmd, *icmd; + int l, cmdlen; IPFW_UH_WLOCK_ASSERT(ch); IPFW_WLOCK_ASSERT(ch); cmd = ACTION_PTR(rule); + l = rule->cmd_len - rule->act_ofs; + while (l > 0) { + cmdlen = F_LEN(cmd); + l -= cmdlen; + if (cmd->opcode == O_EXTERNAL_ACTION) + break; + cmd += cmdlen; + } + /* + * Return if there is not O_EXTERNAL_ACTION or its id is + * different. + */ if (cmd->opcode != O_EXTERNAL_ACTION || cmd->arg1 != eaction_id) return (0); - - if (instance_id != 0 && rule->act_ofs < rule->cmd_len - 1) { + /* + * If instance_id is specified, we need to truncate the + * rule length. Check if there is O_EXTERNAL_INSTANCE opcode. + */ + if (instance_id != 0 && l > 0) { + MPASS(cmdlen == 1); icmd = cmd + 1; if (icmd->opcode != O_EXTERNAL_INSTANCE || icmd->arg1 != instance_id) return (0); - /* FALLTHROUGH */ + /* + * Since named_object related to this instance will be + * destroyed, truncate the chain of opcodes to remove + * the rest of cmd chain just after O_EXTERNAL_ACTION + * opcode. + */ + EACTION_DEBUG("truncate rule %d: len %u -> %u", + rule->rulenum, rule->cmd_len, rule->cmd_len - l); + rule->cmd_len -= l; + MPASS(((uint32_t *)icmd - + (uint32_t *)rule->cmd) == rule->cmd_len); } cmd->arg1 = default_id; /* Set to default id */ - /* - * Since named_object related to this instance will be - * also destroyed, truncate the chain of opcodes to - * remove the rest of cmd chain just after O_EXTERNAL_ACTION - * opcode. - */ - if (rule->act_ofs < rule->cmd_len - 1) { - EACTION_DEBUG("truncate rule %d: len %u -> %u", - rule->rulenum, rule->cmd_len, rule->act_ofs + 1); - rule->cmd_len = rule->act_ofs + 1; - } /* * Return 1 when reset successfully happened. */ -- cgit v1.2.3