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

1 files changed, 30 insertions, 14 deletions

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,36 +377,52 @@ 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.
 	 */
 	return (1);