git: 3b67473b9757 - main - ipfw: add additional handling for orphaned states

The branch main has been updated by ae:

URL: https://cgit.FreeBSD.org/src/commit/?id=3b67473b97574d13eef8302c61c7245b3b3c52c1

commit 3b67473b97574d13eef8302c61c7245b3b3c52c1
Author:     Andrey V. Elsukov <ae@FreeBSD.org>
AuthorDate: 2025-07-22 08:12:36 +0000
Commit:     Andrey V. Elsukov <ae@FreeBSD.org>
CommitDate: 2025-08-03 09:54:39 +0000

    ipfw: add additional handling for orphaned states
    
    When parent rule of dynamic state is deleted and
    net.inet.ip.fw.dyn_keep_states is enabled, dynamic states are kept
    working and such states are called ORPHANED.
    Orphaned states still keep pointer to original parent rule. And in
    case when rule action is skipto this can lead to unpredictable
    consequences. To avoid this problem add special handling for skipto
    action when we have found ORPHANED state.
    Check that new rule has the same opcode and skipto number for
    O_SKIPTO rule action.
    
    Obtained from:  Yandex LLC
    Sponsored by:   Yandex LLC
    Differential Revision: https://reviews.freebsd.org/D51459
---
 sys/netpfil/ipfw/ip_fw_dynamic.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/sys/netpfil/ipfw/ip_fw_dynamic.c b/sys/netpfil/ipfw/ip_fw_dynamic.c
index 40598cef8076..9694c145e112 100644
--- a/sys/netpfil/ipfw/ip_fw_dynamic.c
+++ b/sys/netpfil/ipfw/ip_fw_dynamic.c
@@ -1323,6 +1323,33 @@ dyn_lookup_ipv6_parent_locked(const struct ipfw_flow_id *pkt, uint32_t zoneid,
 
 #endif /* INET6 */
 
+static int
+dyn_handle_orphaned(struct ip_fw *old_rule, struct dyn_data *data)
+{
+	struct ip_fw *rule;
+	const ipfw_insn *cmd, *old_cmd;
+
+	old_cmd = ACTION_PTR(old_rule);
+	switch (old_cmd->opcode) {
+	case O_SETMARK:
+	case O_SKIPTO:
+		/*
+		 * Rule pointer was changed. For O_SKIPTO action it can be
+		 * dangerous to keep use old rule. If new rule has the same
+		 * action and the same destination number, then use this dynamic
+		 * state. Otherwise it is better to create new one.
+		 */
+		rule = V_layer3_chain.map[data->f_pos];
+		cmd = ACTION_PTR(rule);
+		if (cmd->opcode != old_cmd->opcode ||
+		    cmd->len != old_cmd->len || cmd->arg1 != old_cmd->arg1 ||
+		    insntoc(cmd, u32)->d[0] != insntoc(old_cmd, u32)->d[0])
+			return (-1);
+		break;
+	}
+	return (0);
+}
+
 /*
  * Lookup dynamic state.
  *  pkt - filled by ipfw_chk() ipfw_flow_id;
@@ -1426,8 +1453,13 @@ ipfw_dyn_lookup_state(const struct ip_fw_args *args, const void *ulp,
 				 * changed to point to the penultimate rule.
 				 */
 				MPASS(V_layer3_chain.n_rules > 1);
-				data->chain_id = V_layer3_chain.id;
-				data->f_pos = V_layer3_chain.n_rules - 2;
+				if (dyn_handle_orphaned(rule, data) == 0) {
+					data->chain_id = V_layer3_chain.id;
+					data->f_pos = V_layer3_chain.n_rules - 2;
+				} else {
+					rule = NULL;
+					info->direction = MATCH_NONE;
+				}
 			} else {
 				rule = NULL;
 				info->direction = MATCH_NONE;