Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 29 Jul 2019 15:09:12 +0000 (UTC)
From:      "Andrey V. Elsukov" <ae@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r350417 - head/sys/netpfil/ipfw
Message-ID:  <201907291509.x6TF9CGc000401@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: ae
Date: Mon Jul 29 15:09:12 2019
New Revision: 350417
URL: https://svnweb.freebsd.org/changeset/base/350417

Log:
  dd ipfw_get_action() function to get the pointer to action opcode.
  
  ACTION_PTR() returns pointer to the start of rule action section,
  but rule can keep several rule modifiers like O_LOG, O_TAG and O_ALTQ,
  and only then real action opcode is stored.
  
  ipfw_get_action() function inspects the rule action section, skips
  all modifiers and returns action opcode.
  
  Use this function in ipfw_reset_eaction() and flush_nat_ptrs().
  
  MFC after:	1 week
  Sponsored by:	Yandex LLC

Modified:
  head/sys/netpfil/ipfw/ip_fw_eaction.c
  head/sys/netpfil/ipfw/ip_fw_nat.c
  head/sys/netpfil/ipfw/ip_fw_private.h
  head/sys/netpfil/ipfw/ip_fw_sockopt.c

Modified: head/sys/netpfil/ipfw/ip_fw_eaction.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_eaction.c	Mon Jul 29 14:59:14 2019	(r350416)
+++ head/sys/netpfil/ipfw/ip_fw_eaction.c	Mon Jul 29 15:09:12 2019	(r350417)
@@ -377,33 +377,30 @@ ipfw_reset_eaction(struct ip_fw_chain *ch, struct ip_f
     uint16_t eaction_id, uint16_t default_id, uint16_t instance_id)
 {
 	ipfw_insn *cmd, *icmd;
-	int l, cmdlen;
+	int l;
 
 	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 || l <= 0)
-			break;
-		cmd += cmdlen;
-	}
 	/*
 	 * Return if there is not O_EXTERNAL_ACTION or its id is
 	 * different.
 	 */
+	cmd = ipfw_get_action(rule);
 	if (cmd->opcode != O_EXTERNAL_ACTION ||
 	    cmd->arg1 != eaction_id)
 		return (0);
 	/*
 	 * If instance_id is specified, we need to truncate the
 	 * rule length. Check if there is O_EXTERNAL_INSTANCE opcode.
+	 *
+	 * NOTE: F_LEN(cmd) must be 1 for O_EXTERNAL_ACTION opcode,
+	 *  and rule length should be enough to keep O_EXTERNAL_INSTANCE
+	 *  opcode, thus we do check for l > 1.
 	 */
-	if (instance_id != 0 && l > 0) {
-		MPASS(cmdlen == 1);
+	l = rule->cmd + rule->cmd_len - cmd;
+	if (instance_id != 0 && l > 1) {
+		MPASS(F_LEN(cmd) == 1);
 		icmd = cmd + 1;
 		if (icmd->opcode != O_EXTERNAL_INSTANCE ||
 		    icmd->arg1 != instance_id)
@@ -415,8 +412,9 @@ ipfw_reset_eaction(struct ip_fw_chain *ch, struct ip_f
 		 * opcode.
 		 */
 		EACTION_DEBUG("truncate rule %d: len %u -> %u",
-		    rule->rulenum, rule->cmd_len, rule->cmd_len - l);
-		rule->cmd_len -= l;
+		    rule->rulenum, rule->cmd_len,
+		    rule->cmd_len - F_LEN(icmd));
+		rule->cmd_len -= F_LEN(icmd);
 		MPASS(((uint32_t *)icmd -
 		    (uint32_t *)rule->cmd) == rule->cmd_len);
 	}

Modified: head/sys/netpfil/ipfw/ip_fw_nat.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_nat.c	Mon Jul 29 14:59:14 2019	(r350416)
+++ head/sys/netpfil/ipfw/ip_fw_nat.c	Mon Jul 29 15:09:12 2019	(r350417)
@@ -140,13 +140,12 @@ ifaddr_change(void *arg __unused, struct ifnet *ifp)
 static void
 flush_nat_ptrs(struct ip_fw_chain *chain, const int ix)
 {
-	int i;
 	ipfw_insn_nat *cmd;
+	int i;
 
 	IPFW_WLOCK_ASSERT(chain);
 	for (i = 0; i < chain->n_rules; i++) {
-		cmd = (ipfw_insn_nat *)ACTION_PTR(chain->map[i]);
-		/* XXX skip log and the like ? */
+		cmd = (ipfw_insn_nat *)ipfw_get_action(chain->map[i]);
 		if (cmd->o.opcode == O_NAT && cmd->nat != NULL &&
 			    (ix < 0 || cmd->nat->id == ix))
 			cmd->nat = NULL;

Modified: head/sys/netpfil/ipfw/ip_fw_private.h
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_private.h	Mon Jul 29 14:59:14 2019	(r350416)
+++ head/sys/netpfil/ipfw/ip_fw_private.h	Mon Jul 29 15:09:12 2019	(r350417)
@@ -665,6 +665,7 @@ struct ip_fw *ipfw_alloc_rule(struct ip_fw_chain *chai
 void ipfw_free_rule(struct ip_fw *rule);
 int ipfw_match_range(struct ip_fw *rule, ipfw_range_tlv *rt);
 int ipfw_mark_object_kidx(uint32_t *bmask, uint16_t etlv, uint16_t kidx);
+ipfw_insn *ipfw_get_action(struct ip_fw *);
 
 typedef int (sopt_handler_f)(struct ip_fw_chain *ch,
     ip_fw3_opheader *op3, struct sockopt_data *sd);

Modified: head/sys/netpfil/ipfw/ip_fw_sockopt.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_sockopt.c	Mon Jul 29 14:59:14 2019	(r350416)
+++ head/sys/netpfil/ipfw/ip_fw_sockopt.c	Mon Jul 29 15:09:12 2019	(r350417)
@@ -1218,6 +1218,35 @@ move_range(struct ip_fw_chain *chain, ipfw_range_tlv *
 }
 
 /*
+ * Returns pointer to action instruction, skips all possible rule
+ * modifiers like O_LOG, O_TAG, O_ALTQ.
+ */
+ipfw_insn *
+ipfw_get_action(struct ip_fw *rule)
+{
+	ipfw_insn *cmd;
+	int l, cmdlen;
+
+	cmd = ACTION_PTR(rule);
+	l = rule->cmd_len - rule->act_ofs;
+	while (l > 0) {
+		switch (cmd->opcode) {
+		case O_ALTQ:
+		case O_LOG:
+		case O_TAG:
+			break;
+		default:
+			return (cmd);
+		}
+		cmdlen = F_LEN(cmd);
+		l -= cmdlen;
+		cmd += cmdlen;
+	}
+	panic("%s: rule (%p) has not action opcode", __func__, rule);
+	return (NULL);
+}
+
+/*
  * Clear counters for a specific rule.
  * Normally run under IPFW_UH_RLOCK, but these are idempotent ops
  * so we only care that rules do not disappear.

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201907291509.x6TF9CGc000401>